GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||
GeezerVison is showing github.com — Make every API-sourced enum forward-compatible, and detect drift proactively by lelia · Pull Request #107 · SocketDev/socket-sdk-python Conversation
|
| Enum | Fallback | Why not something else |
|---|---|---|
SocketIssueSeverity |
new UNKNOWN |
LOW hides a serious finding, CRITICAL manufactures one |
DiffType |
new UNKNOWN |
UNCHANGED would silently drop a real diff entry |
ScanType |
new UNKNOWN |
no safe existing default |
SocketCategory |
MISCELLANEOUS |
matches the existing convention |
SecurityAction |
DEFER |
already means "use the configured default"; IGNORE disables a rule, ERROR fails builds |
DEFER is inferred from the action descriptions in the OpenAPI spec rather than from backend code. If defer does not mean "fall through to the org default" in every context this enum is parsed from, a new UNKNOWN sentinel would be the safer choice.
tests/unit/test_enum_forward_compat.py walks the package, discovers every enum including ones added later, and fails if any raises on an unrecognized value. It also asserts the fallback is a real member of its own enum, that the warning names the enum, and that known values still round-trip.
Two details that matter: a test_enums_are_discovered guard prevents a broken walk from making every other assertion vacuously pass, and a documented REQUEST_ONLY_ENUMS opt-out (currently empty) gives a future request-building enum a legitimate escape hatch — strictness is correct where the value comes from the caller, not the API.
scripts/check_api_enum_drift.py compares the SDK's enums against https://api.socket.dev/v0/openapi. That spec is public and unauthenticated, so this needs no token, no org and no fixture data.
It immediately found ten purl types the API defines and this SDK did not know about, all of which were being flattened to unknown: alpm, chrome, clawhub, edge-extension, firefox-extension, qpkg, socket, swid, vscode, vscode-extension. Those are added here.
Missing values fail. SDK-only values warn rather than fail, since dropping a member would be breaking and the spec omitting one is usually a spec gap. The two enums the spec exposes no named schema for (ScanType, SecurityAction) are reported as uncovered rather than silently skipped.
.github/workflows/api-drift-check.yml runs it. It is workflow_dispatch-only for now, with the weekly schedule commented and a TODO to enable once it has run green by hand a few times. Deliberately not a pull_request trigger: it tests the API, not the diff, and a backend change must never block an unrelated SDK pull request.
.gitattributes plus a one-time line-ending normalization. Eleven Python files were committed with CRLF, socketdev/__init__.py among them, so any tooling that rewrites one converts it to LF and turns a two-line edit into a whole-file diff. That happened while writing this branch. Isolated in its own commit and reviewable with git diff -w, which shows only .gitattributes — no file content changed.ruff check clean on all touched filesNote
Medium Risk
Changes how unknown severities, diff types, categories, and security actions are represented at parse time (fallbacks instead of errors), which affects downstream reporting and policy behavior until the SDK is updated.
Overview
3.6.0 hardens the SDK against API enum drift so a single unknown value no longer fails entire scan/diff parses (the failure mode behind issue #78 and unknown purl types).
Response-parsed enums (SocketIssueSeverity, SocketCategory, DiffType, SecurityAction, and SocketPURL_Type) now use shared _missing_ handling via socketdev/core/enums.py: log a warning and return a documented fallback (UNKNOWN for severity/diff, MISCELLANEOUS for category, DEFER for security actions, UNKNOWN for purl types). ScanType stays strict because it is only sent on create-scan requests. Ten missing SocketPURL_Type members from the live API are added so those artifacts are no longer coerced to unknown. SocketAlert.from_dict drops its bespoke category try/except in favor of enum _missing_.
Guardrails: tests/unit/test_enum_forward_compat.py discovers all package enums and enforces tolerant parsing (with an explicit request-only opt-out for ScanType). scripts/check_api_enum_drift.py compares enums to the public OpenAPI spec; .github/workflows/api-drift-check.yml runs it on manual dispatch only (not on PRs). .gitattributes pins LF line endings to stop CRLF-only noise in future diffs. Version bumped to 3.6.0 with changelog updates.
Reviewed by Cursor Bugbot for commit 2d72644. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Five of the six enums populated from API responses coerced strictly, so a value the backend added would raise ValueError inside from_dict and empty an entire response rather than degrade one field. That is the mechanism behind issue #78 and the unknown `generic` purl type; each was fixed on the single enum that fired, leaving the rest holding the same landmine. All five now fall back to a documented member and log the unrecognized value. The fallbacks are chosen rather than convenient: SocketIssueSeverity and DiffType gain an explicit UNKNOWN, since guessing an existing level would either hide a real finding or invent one, and SecurityAction defers. A generalized test discovers every enum in the package, including ones added later, and fails if any raises. A drift check compares the enums against the public OpenAPI spec; it found 10 purl types the SDK was missing, which are added here. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Ticket identifiers belong in the pull request description, not in code that outlives the ticket. The GitHub issue reference in the purl test docstring stays, since that resolves for anyone reading the repository. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Eleven Python files were committed with CRLF, socketdev/__init__.py among them. Any tooling that reads and rewrites one of those files converts it to LF on the way out, so a two-line edit arrives as a whole-file diff with the real change buried in it. That happened while writing the enum change in this same branch. This normalizes all of them once and pins the setting so it cannot recur. Reviewable with `git diff -w`, which shows only .gitattributes: no file content changed, and the unit suite is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
There was an error while loading. Please reload this page.
ScanType never parses an API response. FullScanParams.to_dict() is urlencoded onto the create-scan query string, so giving it a _missing_ fallback meant a caller typo silently shipped scan_type=unknown to the API instead of failing at construction. The same from_dict already passes integration_type through uncoerced for that reason. It is now recorded in REQUEST_ONLY_ENUMS, the opt-out the invariant test always had and this branch had left empty, and a new test asserts request-only enums keep raising so the exemption cannot quietly become a skip. Also bumps actions/setup-python in the new workflow to v7.0.0, matching the pin already used by .github/actions/setup-sfw. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
|
bugbot run |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2d72644. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
[agent] Read the real diff (the raw stat is inflated by an isolated CRLF-to-LF normalization commit, isolated cleanly per the PR description and verified with git diff -w) -- enum definitions, the shared _missing_ helper, the new invariant test, and the drift-check script.
Every fallback is deliberately chosen and justified inline rather than defaulted to something convenient (DEFER for SecurityAction because it already means 'use the org default', a new UNKNOWN for severity/diff-type rather than guessing an existing level). ScanType correctly stays strict since it's outbound-only, and the commit history shows that got caught and fixed before this landed, with a test that now actively asserts it stays strict rather than just not-currently-tolerant.
test_enum_forward_compat.py enforcing this generically across the whole package, including future enums, rather than adding a fourth bespoke regression test, is the right level of fix for a bug class that's already recurred twice. The drift script's scope (public spec, manual-dispatch only, not a pull_request trigger) is exactly right for what it's testing.
No findings.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.