fix(wizard): component_type 422, existing sbomify.json, edge-case hardening + review fixes#240
Merged
vpetersson merged 6 commits intoJun 1, 2026
Conversation
Five bugs surfaced by end-to-end wizard testing against a live backend,
plus two follow-ups from adversarial review.
P0 component_type "sbom" is not a valid backend enum value ({bom, document})
— it 422'd on every wizard onboarding and Yocto upload. Switch the three
call sites to "bom" and add a VALID_COMPONENT_TYPES guard in
create_component that fails fast (ValueError) on an unknown literal rather
than letting it become an opaque server 422.
P1 The wizard ignored a pre-existing repo-root sbomify.json: the config form
opened blank and apply then refused to overwrite (no wizard sentinel),
dead-ending the run. Detect it (RepoFacts.has_sbomify_json), pre-fill the
form from disk, and have apply warn-and-continue on the ownership check
(the file is kept and read by the action at run time) so the workflow and
components still get created.
P2 - authenticate: correct the misleading "Token spans N workspaces" copy —
/workspaces returns all of the user's workspaces regardless of token
scope (the backend does not filter that endpoint).
- sbomify_api: collapse pydantic 422 list-detail into readable
"<field>: <msg>" text (clean_validation_error, module-level) instead of
a raw dict repr; wired into the client error paths and the upload
destination.
- generation: add run_command(log_errors=) so cdxgen's expected fallback
failures log at DEBUG, not ERROR, on the happy path.
Plan-limit detection keys off the raw string detail (not the cleaned text) so
a 403 validation list can't be misclassified as PlanLimitError.
Regression tests added for each fix; full suite 2307 passed.
Four low-severity robustness/consistency fixes found by a follow-up audit of the action subsystems (each with a regression test): - enrichment: has_data() now counts cle_release_date — a source returning only a release date was wrongly treated as empty and silently discarded (metadata.py). - dependency-track upload: catch the base requests.RequestException, not just ConnectionError/Timeout, so an SSLError/ProxyError returns a clean UploadResult.failure_result instead of propagating and crashing the step. - cyclonedx-cargo generator: add the check_tool_available guard its cyclonedx-py / syft siblings already have, so supports() returns False when cargo-cyclonedx isn't installed (pip installs) — avoids a spurious ERROR and a wasted attempt before the orchestrator falls through. - chainguard: read manifest/layer "digest" via .get() instead of direct indexing, so a malformed/non-spec OCI manifest yields a graceful None rather than an uncaught KeyError. Full suite 2312 passed.
The mypy hook runs in an isolated env (pass_filenames: false, additional_dependencies only). It omitted textual, so mypy saw the wizard's Textual base classes (App/Screen/widgets) as Any and emitted 10 spurious "Class cannot subclass value of type Any" / "Returning Any" errors — the hook failed on every commit even though `uv run mypy sbomify_action` (textual installed) reports "Success: no issues found". Adding textual>=0.85.0 to the hook deps resolves all 10 at the root: no code changes, no type: ignore. Pre-commit now passes mypy, ruff, and ruff-format.
Follow-ups from the multi-dimensional review of the wizard/hardening changes: - apply.py: the sbomify.json ownership-skip warning now states explicitly that values entered in the wizard were NOT written (the form seeds from a hand-authored file, so a silent skip was a data-loss surprise); and document why a workflow ownership conflict stays fatal while json_config only warns. - utils.run_command: log_errors now also covers the timeout and missing-binary branches, so a cdxgen timeout on a Python lockfile no longer spams ERROR when a later generator succeeds (previously only the non-zero-exit path was gated). - tests: pin the apply.py component_type="bom" call-site directly; cover _load_from_disk's malformed/non-dict error paths (form opens blank, no crash); cover the chainguard provenance missing-digest path. - polish: pin the mypy hook's textual to ==8.2.7 (uv.lock version) for a reproducible type view; hoist _load_from_disk's local imports to module top; isinstance(... list | tuple) idiom; trailing newline in .pre-commit-config. Full suite 2315 passed; pre-commit (ruff, ruff-format, mypy) green.
The module docstring said apply overwrites "after .bak backup", but io.py deliberately writes no .bak (git is the source of truth). Align the contract text with io.py's actual behavior. Addresses a PR sbomify#239 review comment.
Scoped to the code this fork PR's fix commits touched (the feature-code comments are tracked on the canonical sbomify#238): - apply.py: surface the unwritten sbomify.json payload in the apply log when the ownership check skips a hand-authored file, so the values entered in the wizard are recoverable without re-running it. - apply.py: correct the apply_plan docstring's stale ".bak backup" claim to match io.py (sentinel-guarded overwrite, no .bak; git holds prior versions). - chainguard.py: comment the .get("digest") continue-on-missing semantics in _detect_chainguard_from_provenance (mirrors _resolve_platform_digest). Full suite 2315 passed; ruff/format/mypy green.
vpetersson
approved these changes
Jun 1, 2026
vpetersson
merged commit 7b01da1
into
sbomify:feat/wizard-and-shared-api-client
9 checks passed
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
2 participants
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.
Summary
Fixes on top of the wizard + consolidated API client feature, targeting its branch (
feat/wizard-and-shared-api-client) directly so the diff is just these 6 commits, not the whole feature. (Supersedes #239, which was opened againstmasterand so duplicated the entire feature / #238.)These can be merged into #238's branch, or cherry-picked onto it.
Commits
dc4b02cfix(wizard): valid component_type, existing sbomify.json, error polishcomponent_type"sbom"→"bom"— the backendComponentTypeenum is{bom, document};"sbom"returned HTTP 422 on every wizard onboarding and Yocto upload. Adds a client-sideVALID_COMPONENT_TYPESguard so a bad literal fails fast instead of as an opaque 422.sbomify.json, and apply no longer dead-ends on a hand-authored one (keeps it as-is — the action reads it at run time — and logs the unwritten values so they're recoverable).field: msg);run_command(log_errors=)so cdxgen's expected fallback failures don't spam ERROR; corrected the misleading token-scope warning copy.cee6d02fix: harden enrichment, dtrack, cargo, chainguard edge cases —has_data()countscle_release_date; dtrack catches baseRequestException; cargo generator gets thecheck_tool_availableguard its siblings have; chainguard readsdigestvia.get().55f1a1bci: add textual to mypy pre-commit deps so the hook passes — the mypy hook's isolated env was missingtextual, so it saw the wizard's Textual base classes asAnyand failed on every commit (in-venv mypy was already clean). Pinned to==8.2.7(the uv.lock version).f29d96drefactor: address comprehensive-review findings — explicit warning when wizard edits aren't written to a hand-authoredsbomify.json;log_errorsalso covers timeout/missing-binary; regression tests for the apply call-site,_load_from_diskerror paths, and the chainguard provenance digest path; assorted polish.a2664dddocs(wizard) +c611937fix(wizard) — Copilot-review follow-ups: corrected stale.bakdocstrings to matchio.py; surface the unwrittensbomify.jsonpayload in the apply log; document the chainguard.get("digest")continue-on-missing semantics.Testing
pre-commit run --all-files: ruff, ruff-format, mypy all green.Not included
The
sboms.ymlsource-SBOM coverage regression fromee4f28d(prod + JS self-SBOM jobs replaced by aca-certificateswizard test artifact) is out of scope for these fixes and is tracked separately on #238 for the feature author.