Skip to content

fix(bundler): reject falsy non-mapping requires/provides in manifest from_dict#3661

Merged
mnriem merged 2 commits into
github:mainfrom
jawwad-ali:fix/bundler-manifest-provides-guard
Jul 23, 2026
Merged

fix(bundler): reject falsy non-mapping requires/provides in manifest from_dict#3661
mnriem merged 2 commits into
github:mainfrom
jawwad-ali:fix/bundler-manifest-provides-guard

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

What

BundleManifest.from_dict used data.get("requires") or {} and data.get("provides") or {}. The or {} coerces a falsy non-mapping value ([], '', 0, false) to {} before the isinstance guard runs — so a malformed manifest silently passed validation as one that requires/provides nothing. Only a truthy non-mapping (e.g. provides: "extensions") reached the guard and raised.

The sibling integration guard (added in #3629) already does this correctly with an explicit is not None check; requires/provides were the remaining holes.

Fix

Handle None explicitly (default to {}) and reject every other non-mapping:

requires_raw = data.get("requires")
if requires_raw is None:
    requires_raw = {}
elif not isinstance(requires_raw, dict):
    raise BundlerError("'requires' must be a mapping when present.")

…and the same for provides. Absent fields still parse to the empty default.

Tests

tests/contract/test_manifest_schema.py:

  • test_non_mapping_provides_rejected_including_falsy and test_non_mapping_requires_rejected_including_falsy — parametrized over [], '', 0, False, and a truthy string; all raise after the fix (the falsy ones passed silently before)
  • test_absent_provides_and_requires_do_not_raise_mapping_error — absent (None) optional mappings still parse without tripping the guard

ruff clean; all 28 contract tests pass.


AI-assisted: authored with Claude Code. Verified the falsy-coercion hole against the merged integration guard (#3629) and confirmed fail-before/pass-after.

jawwad-ali and others added 2 commits July 22, 2026 22:10
…from_dict

BundleManifest.from_dict used `data.get("requires") or {}` and
`data.get("provides") or {}`, so a FALSY non-mapping value ([], '', 0,
false) was coerced to {} BEFORE the isinstance guard — a malformed manifest
passed validation as one that requires/provides nothing. Only a truthy
non-mapping (e.g. "extensions") was rejected.

Handle None explicitly (default to {}) and reject every other non-mapping,
matching the sibling 'integration' guard added in github#3629. Absent fields still
parse to the empty default.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

Fixes bundle manifest validation so falsy non-mapping requires and provides values are rejected.

Changes:

  • Defaults only absent or None fields to empty mappings.
  • Adds regression tests covering falsy invalid values and absent fields.
Show a summary per file
File Description
src/specify_cli/bundler/models/manifest.py Corrects mapping validation.
tests/contract/test_manifest_schema.py Adds regression coverage.

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 0a7f288 into github:main Jul 23, 2026
14 checks passed
@mnriem

mnriem commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

mnriem pushed a commit that referenced this pull request Jul 23, 2026
… records (#3666)

load_records and InstalledBundleRecord.from_dict defaulted their list fields
with `data.get(...) or []` BEFORE the isinstance(list) guard, so a FALSY
non-list value (0, '', False, {}) was coerced to [] and the guard became dead
code — a corrupt .specify/bundle-records.json was silently read as "no
bundles"/"no components" instead of raising. Only an absent/None value should
mean empty.

Handle None explicitly and reject every other non-list, mirroring the merged
requires/provides/integration guards (#3629, #3661) and the catalog_config
sibling reader.

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

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mnriem pushed a commit that referenced this pull request Jul 23, 2026
…try.from_dict (#3667)

CatalogEntry.from_dict used `data.get("requires") or {}` and
`data.get("provides") or {}`, so a FALSY non-mapping ([], '', 0, false) was
coerced to {} before the isinstance guard — a corrupt catalog entry passed
silently. Only a truthy non-mapping was rejected.

Handle None explicitly and reject every other non-mapping, mirroring the
merged manifest requires/provides/integration guards (#3629, #3661).

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

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.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