fix(bundler): treat an explicit-null records field as missing, not the text "None" - #4136
Open
jawwad-ali wants to merge 1 commit into
Open
fix(bundler): treat an explicit-null records field as missing, not the text "None"#4136jawwad-ali wants to merge 1 commit into
"None"#4136jawwad-ali wants to merge 1 commit into
Conversation
…one"
`InstalledBundleRecord.from_dict` and `_component_from_dict` read required
fields with `str(data.get(key, "")).strip()`. The `""` default covers only a
MISSING key. A key that is present but JSON `null` — how a hand-edited or
corrupt `.specify/bundle-records.json` spells an empty field — yields
`None`, and `str(None)` is the literal `"None"`. That text is non-empty, so
it sails past the required-field guards that exist to reject such a file:
bundle_id=null -> ACCEPTED: bundle_id='None' version='1.0.0'
version=null -> ACCEPTED: bundle_id='a' version='None'
id=null -> ACCEPTED: components=[('presets', 'None')]
The phantom component then feeds the collateral-protection refcount, so
`components_still_needed()` reports ('presets', 'None') as protected.
Reuse `manifest._text`, the sibling helper whose docstring describes this
exact trap and which fixed the same bug for bundle.yml in github#3798. The
falsy-non-list half of this hardening already landed in this file as github#3666;
the explicit-null half was never mirrored here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
InstalledBundleRecord.from_dictand_component_from_dictread the required fields with:The
""default covers only a missing key. A key that is present but null — how a hand-edited or corrupt.specify/bundle-records.jsonspells an empty field — yieldsNone, andstr(None)is the literal"None". That text is non-empty, so it sails straight past theif not bundle_id:/if not version:/if not cid:guards that exist precisely to reject such a file.Reproduction on current
main(bf88c9f)The record is accepted as a bundle literally named
Noneat versionNone, contributing a component whose id isNone.That phantom is not inert: it feeds the collateral-protection refcount, so
components_still_needed()reports('presets', 'None')as protected, andinstall_bundle()'sother_trackedset will attribute real components to it.Fix
Reuse
_textfrom the sibling parser in the same package. Its docstring describes this exact trap:This is the second half of a hardening sweep that was already applied here in part: the falsy-non-list guards landed in this file as #3666, and the explicit-null fix landed for
bundle.ymlas #3798 — but was never mirrored intorecords.py. Using the shared helper keeps the two parsers from drifting again.No breaking change. Only null-valued inputs behave differently; every string input is byte-identical.
installed_atis optional, so its null correctly becomes""rather than an error — pinned by a test.Verification
bundle_id='demo' version='1.0.0' comps=[('presets','p1')].tests/unit: no new failures vs a clean-mainbaseline captured onbf88c9f9.uvx ruff@0.15.0 check src tests→ cleanTests sit beside the existing
test_load_records_rejects_record_missing_bundle_id/_missing_version/_rejects_component_missing_id, which cover the missing-key half of the same contract.Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.