Skip to content

fix(env-loaders): strip matching surrounding quotes from .env values - #989

Merged
sonichi merged 2 commits into
sonichi:mainfrom
VasiliyRad:fix/env-loader-strip-quotes
May 22, 2026
Merged

sonichi merged 2 commits into
sonichi:mainfrom
VasiliyRad:fix/env-loader-strip-quotes

Conversation

@VasiliyRad

Copy link
Copy Markdown
Contributor

Why

Three independent .env loaders share the same naive val.strip()-only pattern, so a properly-formed line like X_API_KEY=\"abc\" ends up as '\"abc\"' (with literal quotes) in os.environ. Then:

  • x-post.py: OAuth1 signs with the quoted key → X returns 401.
  • telegram-bridge.py: URL becomes https://api.telegram.org/bot\"abc\"/getUpdates → Telegram returns 404.
  • generate.py: Gemini Authorization header gets a malformed key.

Cryptic upstream errors with no surface clue that quoting was the cause. Quoted .env values are a standard convention (python-dotenv handles them); these naive loaders don't.

Fix

Mirror python-dotenv exactly: strip ONLY matching surrounding quotes. A 3-line check after the existing val.strip():

```python
if len(val) >= 2 and val[0] == val[-1] and val[0] in ('"', "'"):
val = val[1:-1]
```

Handles every common .env shape:

Input Output
KEY=\"value\" value
KEY='value' value
KEY=value value (unchanged)
KEY=\"abc \"abc (mismatched — left intact)
KEY=\"a\"b\" a\"b (outer pair stripped; inner preserved)

Alternative considered: extract to a shared src/env_loader.py module. Rejected for now because the call sites span src/ (core) and skills/ (which by design avoid importing from src/ for self-containment). The existing structural test catches drift between the 3 copies.

Tests

tests/env-loader.test.py — extended with the quote-strip invariant. Greps each loader for a matching-quote-strip pattern (val[0] == val[-1] or v[0] == v[-1]). All 3 loaders pass against the new check.

Pre-fix: a structural grep of any of the 3 files would NOT match → test fails.
Post-fix: all 3 contain the pattern → test passes.

Test plan

  • python3 tests/env-loader.test.py — all 3 loaders pass extended invariant
  • Manual: a user with X_API_KEY=\"abc\" in .env runs python3 skills/x-twitter/x-post.py search foo → verify X returns results, not 401

🤖 Generated with Claude Code

## Why

Three independent .env loaders (`src/telegram-bridge.py`,
`skills/x-twitter/x-post.py`, `skills/image-generation/scripts/generate.py`)
parse .env lines with the same pattern:

    for line in env_path.read_text().splitlines():
        if "=" in line and not line.startswith("#"):
            key, _, val = line.partition("=")
            os.environ[key.strip()] = val.strip()

This handles unquoted lines correctly (`KEY=value`). It does NOT
handle quoted lines: `KEY="value"` -> `os.environ["KEY"]` becomes
`'"value"'` (with literal quotes). The quoted form is a common .env
convention.

Each loader's downstream consumer then uses the quoted string
verbatim:

  - x-post.py: OAuth1 signature base string includes the quotes ->
    X returns 401.
  - telegram-bridge.py: URL becomes
    `https://api.telegram.org/bot"abc"/getUpdates` -> Telegram
    returns 404.
  - generate.py: Gemini Authorization header gets a malformed key.

## Fix

Mirror python-dotenv's behavior: strip ONLY matching surrounding
quotes. A 3-line check after the existing `val.strip()`:

    if len(val) >= 2 and val[0] == val[-1] and val[0] in ('"', "'"):
        val = val[1:-1]

Handles every common .env shape:

  - KEY="value"   -> value
  - KEY='value'   -> value
  - KEY=value     -> value (unchanged)
  - KEY="abc      -> "abc  (mismatched — left intact)
  - KEY="a"b"     -> a"b   (outer pair stripped; inner preserved)

Alternative considered: extract to a shared `src/env_loader.py`.
Rejected for now because the call sites span `src/` (core) and
`skills/` (which by design avoid importing from `src/` for
self-containment). A shared module would require sys.path tweaks in
each skill that adopts it, undoing the skill-isolation contract.

## Tests

`tests/env-loader.test.py` — extended with the quote-strip invariant.
Greps each loader for the matching-quote-strip pattern (`val[0] ==
val[-1]` or `v[0] == v[-1]`). All 3 loaders pass against the new
check.

Pre-fix: structural grep of any of the 3 files would NOT match the
pattern -> test fails.
Post-fix: all 3 contain the pattern -> test passes.

Co-Authored-By: Vasiliy's Sutando <noreply@anthropic.com>
@CLAassistant

CLAassistant commented May 21, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@sonichi

sonichi commented May 21, 2026

Copy link
Copy Markdown
Owner

LGTM. The mismatched-quote behavior table is the part that makes this correct: KEY="abc → "abc (left intact). A naive "strip leading/trailing "" would mangle that case, which matters for legitimate values that happen to start with a quote — and matters more for the KEY="a"b" case (only outer pair stripped, inner preserved).

The 3-line check (len >= 2 and val[0] == val[-1] and val[0] in ('"', "'")) mirrors python-dotenv's actual behavior, not a half-implementation. The "alternative considered" note rejecting extraction to a shared src/env_loader.py is the right call given the self-containment boundary between src/ and skills/ — a structural grep test catches drift between the 3 copies, which is the pragmatic move.

The visible symptoms were upstream errors with zero surface clue (X 401, Telegram 404, Gemini malformed-auth-header) — exactly the kind of bug that wastes hours when it first hits. Worth landing.

@sonichi
sonichi merged commit fb75344 into sonichi:main May 22, 2026
2 checks passed
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…onichi#989)

## Why

Three independent .env loaders (`src/telegram-bridge.py`,
`skills/x-twitter/x-post.py`, `skills/image-generation/scripts/generate.py`)
parse .env lines with the same pattern:

    for line in env_path.read_text().splitlines():
        if "=" in line and not line.startswith("#"):
            key, _, val = line.partition("=")
            os.environ[key.strip()] = val.strip()

This handles unquoted lines correctly (`KEY=value`). It does NOT
handle quoted lines: `KEY="value"` -> `os.environ["KEY"]` becomes
`'"value"'` (with literal quotes). The quoted form is a common .env
convention.

Each loader's downstream consumer then uses the quoted string
verbatim:

  - x-post.py: OAuth1 signature base string includes the quotes ->
    X returns 401.
  - telegram-bridge.py: URL becomes
    `https://api.telegram.org/bot"abc"/getUpdates` -> Telegram
    returns 404.
  - generate.py: Gemini Authorization header gets a malformed key.

## Fix

Mirror python-dotenv's behavior: strip ONLY matching surrounding
quotes. A 3-line check after the existing `val.strip()`:

    if len(val) >= 2 and val[0] == val[-1] and val[0] in ('"', "'"):
        val = val[1:-1]

Handles every common .env shape:

  - KEY="value"   -> value
  - KEY='value'   -> value
  - KEY=value     -> value (unchanged)
  - KEY="abc      -> "abc  (mismatched — left intact)
  - KEY="a"b"     -> a"b   (outer pair stripped; inner preserved)

Alternative considered: extract to a shared `src/env_loader.py`.
Rejected for now because the call sites span `src/` (core) and
`skills/` (which by design avoid importing from `src/` for
self-containment). A shared module would require sys.path tweaks in
each skill that adopts it, undoing the skill-isolation contract.

## Tests

`tests/env-loader.test.py` — extended with the quote-strip invariant.
Greps each loader for the matching-quote-strip pattern (`val[0] ==
val[-1]` or `v[0] == v[-1]`). All 3 loaders pass against the new
check.

Pre-fix: structural grep of any of the 3 files would NOT match the
pattern -> test fails.
Post-fix: all 3 contain the pattern -> test passes.

Co-authored-by: Vasiliy's Sutando <noreply@anthropic.com>
Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.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.

3 participants