Skip to content

fix(extensions): guard the required manifest sections so one bad extension cannot break extension list - #3797

Open
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/extension-manifest-section-guards
Open

fix(extensions): guard the required manifest sections so one bad extension cannot break extension list#3797
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/extension-manifest-section-guards

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Problem

ExtensionManifest.REQUIRED_FIELDS only checks key presence:

for field in self.REQUIRED_FIELDS:      # schema_version, extension, requires, provides
    if field not in self.data:
        raise ValidationError(...)

So a section that is written but left empty (provides:None, which is just how YAML spells an empty field) or given the wrong shape passes that check and then fails on first use:

extension: null  -> TypeError: argument of type 'NoneType' is not iterable
requires:  null  -> TypeError: argument of type 'NoneType' is not iterable
provides:  null  -> AttributeError: 'NoneType' object has no attribute 'get'
provides:  []    -> AttributeError: 'list' object has no attribute 'get'

Neither is a ValidationError, so both escape the callers that already handle malformed manifests. list_installed() catches ValidationError only and has a deliberate ⚠️ Corrupted extension fallback — so a single bad extension took down the entire command. Reproduced end-to-end with one broken and one good extension installed:

specify extension list
before exit 1, raw AttributeError, no output at all
after exit 0 — the good extension is listed, the bad one shows as Corrupted extension

Fix

Add an isinstance guard for each required section. This mirrors the guards already in the same function for the nested optional keys —

if "commands" in provides and not isinstance(commands, list):
    raise ValidationError("Invalid provides.commands: expected a list")
if "hooks" in self.data and not isinstance(hooks, dict):
    raise ValidationError("Invalid hooks: expected a mapping")

— and _load_yaml's document-root check. Only the three required sections lacked one; they were the odd ones out.

Not a behaviour change for valid input

provides: {} is a well-shaped mapping, so the new check does not touch it:

  • with hooks present → still validates (an extension may provide only hooks)
  • with no hooks → still gets the pre-existing must provide at least one command or hook

Both are locked by regression tests. I got this wrong on the first attempt — my initial test asserted provides: {} must raise, which failed because the fixture defines hooks; the tests now assert the correct behaviour.

Verification

  • test_required_section_not_mapping_rejected — 3 sections × 3 bad shapes (None, [], "text"): 9 fail before, all pass after.
  • 2 regression guards for the provides: {} cases above.
  • TestExtensionManifest: 55 passed.
  • uvx ruff@0.15.0 check src tests clean.

AI-assisted: authored with Claude Code. I reproduced the extension list exit-1 crash end-to-end (correct .registry layout, one broken + one good extension) and confirmed the graceful fallback now engages, rather than only testing the manifest in isolation.

@jawwad-ali
jawwad-ali requested a review from mnriem as a code owner July 28, 2026 14:20
…nsion cannot break `extension list`

ExtensionManifest.REQUIRED_FIELDS only checks key PRESENCE, so a section that is
written but left empty (`provides:` -> None) or given the wrong shape
(`provides: []`) passes it and then fails on first use:

    extension: null  -> TypeError: argument of type 'NoneType' is not iterable
    requires:  null  -> TypeError: argument of type 'NoneType' is not iterable
    provides:  null  -> AttributeError: 'NoneType' object has no attribute 'get'
    provides:  []    -> AttributeError: 'list' object has no attribute 'get'

Neither is a ValidationError, so both escape the callers that already handle
malformed manifests. list_installed() catches ValidationError only and has a
deliberate "Corrupted extension" fallback, so a single bad extension took down
the whole command -- reproduced end-to-end:

    before: specify extension list -> exit 1, raw AttributeError, no output
    after:  specify extension list -> exit 0, the good extension listed, the
            bad one shown as "Corrupted extension"

Add an isinstance guard for each required section, mirroring the nested guards
already in this function ("Invalid provides.commands: expected a list", "Invalid
hooks: expected a mapping") and _load_yaml's document-root check. Only the three
REQUIRED sections lacked one.

`provides: {}` is unaffected: it is a well-shaped mapping, so an extension that
provides only hooks still validates, and with no hooks it keeps the pre-existing
"must provide at least one command or hook" message. Both are locked by tests.

🤖 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/extension-manifest-section-guards branch from 85b1b67 to e2cf3ee Compare July 28, 2026 14:57
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