Skip to content

fix(bundler): degrade non-UTF-8 config reads into BundlerError - #3784

Merged
mnriem merged 1 commit into
github:mainfrom
jawwad-ali:fix/yamlio-unicode-decode-guard
Jul 28, 2026
Merged

fix(bundler): degrade non-UTF-8 config reads into BundlerError#3784
mnriem merged 1 commit into
github:mainfrom
jawwad-ali:fix/yamlio-unicode-decode-guard

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Problem

bundler/lib/yamlio.py is the single chokepoint for every bundler read, and its module docstring states the contract explicitly:

All reads/writes go through these functions so that IO failures degrade into actionable BundlerErrors rather than raw tracebacks.

Both readers catch only OSError. But Path.read_text(encoding="utf-8") and json.load() raise UnicodeDecodeError on a non-UTF-8 file, and:

issubclass(UnicodeDecodeError, OSError)  # False   (MRO: UnicodeError -> ValueError)

So the decode error escapes uncaught. Reproduced on main with a UTF-16 file:

load_yaml: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff in position 0
load_json: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff in position 0

In load_json, json.JSONDecodeError does not cover it — the two are sibling ValueError subclasses, neither subsumes the other.

This is realistic rather than theoretical: on Windows, PowerShell 5.1's Out-File and > default to UTF-16, so a hand-edited .specify/bundle-catalogs.yml or records file lands here.

Fix

Widen both read clauses to (OSError, UnicodeError) — matching the sibling catalog readers (catalogs.py:101, workflows/catalog.py:336, which already use exactly this tuple).

json.JSONDecodeError deliberately stays first in load_json, so malformed-but-decodable JSON keeps its more specific Invalid JSON message while a decode failure falls through to the read-error clause. A regression test locks that ordering.

Scope: reads only. I verified the write paths do not leak UnicodeEncodeErrordump_yaml/dump_json both escape unencodable input (tested with a lone surrogate), so there is nothing to fix there.

Verification

  • test_load_yaml_non_utf8_raises_bundler_error and test_load_json_non_utf8_raises_bundler_error (UTF-16 input): fail before with the raw UnicodeDecodeError, pass after.
  • test_load_json_malformed_still_reports_invalid_json: clause-order guard — decodable-but-malformed JSON still reports Invalid JSON.
  • tests/unit/test_bundler_yamlio.py: 5 passed.
  • tests/unit/ + tests/contract/: 252 passed (2 pre-existing Windows symlink-privilege failures, unrelated — they need admin and pass on CI's Linux/macOS runners).
  • uvx ruff@0.15.0 check src tests clean.

No behaviour change for UTF-8 input: those reads take the identical path and return identical values.


AI-assisted: authored with Claude Code. I reproduced both leaks on main, checked issubclass rather than assuming, verified the write paths were safe before scoping the fix, and matched the existing sibling-reader tuple.

@jawwad-ali
jawwad-ali requested a review from mnriem as a code owner July 28, 2026 11:21
yamlio.py is the single chokepoint for every bundler read, and its module
docstring states the contract: "All reads/writes go through these functions so
that IO failures degrade into actionable BundlerError rather than raw
tracebacks."

Both readers catch only OSError, but `Path.read_text(encoding="utf-8")` and
`json.load()` raise UnicodeDecodeError on a non-UTF-8 file --
`issubclass(UnicodeDecodeError, OSError)` is False (its MRO is UnicodeError ->
ValueError). So the decode error escaped uncaught:

    load_yaml: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff
    load_json: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff

In load_json, json.JSONDecodeError does not help: it is a *sibling* of
UnicodeDecodeError, not a parent.

This is realistic rather than theoretical -- on Windows, PowerShell 5.1's
`Out-File` and `>` default to UTF-16, so a hand-edited
`.specify/bundle-catalogs.yml` or records file hits it.

Widen both read clauses to `(OSError, UnicodeError)`, matching the sibling
catalog readers (catalogs.py:101, workflows/catalog.py:336). JSONDecodeError
deliberately stays FIRST so malformed-but-decodable JSON keeps its more
specific "Invalid JSON" message; a regression test locks that ordering.

Write paths are unaffected -- verified that dump_yaml/dump_json do not leak
UnicodeEncodeError (both escape unencodable input), so this stays scoped to the
two read paths.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jawwad-ali
jawwad-ali force-pushed the fix/yamlio-unicode-decode-guard branch from 8e22774 to 0c6f70b Compare July 28, 2026 14:56
@mnriem
mnriem requested a review from Copilot July 28, 2026 20:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Handles non-UTF-8 bundler configuration files as actionable BundlerErrors instead of raw decode tracebacks.

Changes:

  • Catch UnicodeError during YAML and JSON reads.
  • Add regression tests for UTF-16 input and malformed JSON messaging.
Show a summary per file
File Description
src/specify_cli/bundler/lib/yamlio.py Converts Unicode read failures into BundlerError.
tests/unit/test_bundler_yamlio.py Tests non-UTF-8 handling and JSON error specificity.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Medium

@mnriem
mnriem merged commit be33d2a into github:main Jul 28, 2026
14 checks passed
@mnriem

mnriem commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

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