Make every API-sourced enum forward-compatible, and detect drift proactively - #107
Open
lelia wants to merge 6 commits into
Open
Make every API-sourced enum forward-compatible, and detect drift proactively#107lelia wants to merge 6 commits into
lelia wants to merge 6 commits into
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
4 tasks
There was a problem hiding this comment.
✅ 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.
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.
Five of the six enums populated from API responses coerced strictly, so any value the backend added would raise
ValueErrorinsidefrom_dictand empty an entire response rather than degrade one field. That is the mechanism behind issue #78 and the unknowngenericpurl type. Each was fixed on the single enum that happened to fire, leaving the other five holding the same landmine.This fixes the class rather than the next instance, and adds a check that finds the next one before a customer does.
Forward-compatible enums
SocketIssueSeverity,SocketCategory,DiffType,ScanTypeandSecurityActionnow fall back to a documented member and log the unrecognized value.SocketPURL_Typealready did; its inline implementation now routes through one shared helper insocketdev/core/enums.pyso all six behave and log identically. The deadtry/exceptinSocketAlert.from_dictis removed, since_missing_covers every coercion path rather than that one call site.The fallbacks are chosen, not convenient:
SocketIssueSeverityUNKNOWNLOWhides a serious finding,CRITICALmanufactures oneDiffTypeUNKNOWNUNCHANGEDwould silently drop a real diff entryScanTypeUNKNOWNSocketCategoryMISCELLANEOUSSecurityActionDEFERIGNOREdisables a rule,ERRORfails buildsWorth a second opinion: SecurityAction
DEFERis inferred from the action descriptions in the OpenAPI spec rather than from backend code. Ifdeferdoes not mean "fall through to the org default" in every context this enum is parsed from, a newUNKNOWNsentinel would be the safer choice.Enforced as an invariant
tests/unit/test_enum_forward_compat.pywalks 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_discoveredguard prevents a broken walk from making every other assertion vacuously pass, and a documentedREQUEST_ONLY_ENUMSopt-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.Drift detection with no setup
scripts/check_api_enum_drift.pycompares the SDK's enums againsthttps://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.ymlruns it. It isworkflow_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 apull_requesttrigger: it tests the API, not the diff, and a backend change must never block an unrelated SDK pull request.Also in here
.gitattributesplus a one-time line-ending normalization. Eleven Python files were committed with CRLF,socketdev/__init__.pyamong 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 withgit diff -w, which shows only.gitattributes— no file content changed.Verification
ruff checkclean 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, andSocketPURL_Type) now use shared_missing_handling viasocketdev/core/enums.py: log a warning and return a documented fallback (UNKNOWNfor severity/diff,MISCELLANEOUSfor category,DEFERfor security actions,UNKNOWNfor purl types).ScanTypestays strict because it is only sent on create-scan requests. Ten missingSocketPURL_Typemembers from the live API are added so those artifacts are no longer coerced tounknown.SocketAlert.from_dictdrops its bespoke categorytry/exceptin favor of enum_missing_.Guardrails:
tests/unit/test_enum_forward_compat.pydiscovers all package enums and enforces tolerant parsing (with an explicit request-only opt-out forScanType).scripts/check_api_enum_drift.pycompares enums to the public OpenAPI spec;.github/workflows/api-drift-check.ymlruns it on manual dispatch only (not on PRs)..gitattributespins 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.