Repository navigation
feat(batch): verify webhook signatures and match the API's result shapes - #55
Draft
emirlan-zen wants to merge 2 commits into
Draft
emirlan-zen wants to merge 2 commits into
emirlan-zen wants to merge 2 commits into
Conversation
Additive: no exported name, field type, signature or accepted input is removed or narrowed. - Add `verify_webhook(raw_body, signature_header, secrets)`. It decodes the base64 secret, picks the key by `kid`, compares in constant time, rejects deliveries more than five minutes old by default (only a tolerance of 0 disables that), and returns False, never raising, on a malformed header, a bad tolerance or `now`, or wrong-typed arguments. A secret that doesn't decode to 32 bytes never verifies, so an empty secret can't be used to forge a signature. - Bulk downloads (`download_to_dir`, `download_to_memory`) skip rows with no body to fetch (failed, `expired` past the visibility window, or not finished yet) and log the counts per status, instead of stopping part-way with a ValueError. A `successful` row without a `result_url` gets its page fetched once more; if the link is still missing, the download raises `ResultURLUnavailable`, a ValueError subclass, so existing handlers still catch it. - `TaskStatus.EXPIRED` for results past the visibility window. - `add_tasks(..., idempotency_key=...)` sends `Idempotency-Key`, so the call is safe to retry and the transport retries it on 429/502/503/504. Without a key it is still sent once. - `Run.trigger` (`manual` or `scheduled`). - `estimate_cost` treats a boolean flag as on only for True or a string equal to "true" ignoring case, as the API does; it used to price 1, "1", "yes" and "on" as on, which the API rejects. - Keep `TaskResult.spend` and `TaskSpend`, documented as deprecated: result rows never carry per-task spend; read `run.stats.spend`. - Docs: no fixed export cap (the server's error reads "results are larger than N gb"); client-side downloads are bound by their own configurable limits; drop the stale `ZENROWS_BATCH_PROGRESS` note and the "extract_fields is not supported" note; per-task params are `zenrows_params`; target 404 and 410 responses are billed; `run.stats.spend` is indicative, not billing-grade; result URLs are valid for 2 hours and are never `/content` URLs (that is a separate, keyed endpoint); webhook delivery is best-effort with bounded retries and nothing is sent on stop. - Refresh docs/openapi.yaml (task limits 100,000 per submit and 10,000 per add call, 429 quota_exceeded and when a job stops counting as active, add-tasks idempotency, the 2,048-byte URL limit) and regenerate the models and the API reference.
This branch has not been deployed
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.
What this changes
This PR brings the Batch client in line with what the Batch API returns, and adds a webhook signature check. It is additive: no exported name, field, type, signature or accepted input is removed or narrowed. See "Compatibility" for how that was checked.
New:
verify_webhook(raw_body, signature_header, secrets)checks theX-Signatureheader (t=...,v1=...,kid=...) on a signed delivery.kid, so it keeps working during a key rotation.<t>.<raw body>and compares in constant time.tolerance_seconds=0disables the age check.now, even with the age check off.kid, an unreadable secrets mapping, a body that can't be encoded and wrong-typed arguments all returnFalse.Bulk downloads no longer stop part-way.
download_to_diranddownload_to_memoryused to raiseValueErroron the first row with noresult_url.expiredpending/processing)zenrows.batch.downloadgives the counts per status. The return value is still the number of rows downloaded.successfulrow with noresult_urlis not skipped. Its body exists, but the API could not sign a link on that read.ResultURLUnavailable(withtask_ids) instead of dropping a result silently.ResultURLUnavailablesubclassesValueError, so an existingexcept ValueErroraround a bulk download still catches it.ValueErrorfor a row with no URL, as before.TaskStatus.EXPIREDis added (an enum widening; see Compatibility).add_tasks(..., idempotency_key=...)is a new optional keyword.Run.trigger(manualorscheduled) is a new optional field. The API already sends it.estimate_costreads boolean flags the way the API does."true"/"false". It rejects1,"1","yes"and"on"with a 400." TRUE ") as on, for jobs the API would refuse.Trueor a string equal to"true"ignoring case turnsjs_render/premium_proxyon.TaskResult.spendandTaskSpendare kept and documented as deprecated.None.run.stats.spendis the run-level figure.README webhook wording. It now says:
/stop.X-ZenRows-Event-Id.It used to say "at-least-once".
Docs and docstrings:
"results are larger than N gb".max_files/max_bytes_per_file.ZENROWS_BATCH_PROGRESSsetting is no longer documented.extract_fieldsworks in Batch.zenrows_params.run.stats.spendis indicative recorded spend; the account's usage and invoices are authoritative./contentis a separate endpoint that needs the API key.Spec refresh in
docs/openapi.yaml:max_length.429 quota_exceeded, including when a job stops counting as active: once it is closed and its latest run has finished, or at once when it is deleted.The models and
docs/batch-client-reference.mdare regenerated, and the reference gains a "Webhook signatures" section.Compatibility
Nothing a current caller relies on changes.
TaskInput.urlandTaskResult.urlstayAnyUrl, so.hostand passing anAnyUrlstill work.TaskSpendandTaskResult.spendstay. Every exported name and every existing signature is unchanged.griffe check zenrows -s src --against origin/mainreports no findings: it prints nothing and exits 0.Widenings and behaviour changes griffe does not see, disclosed here:
TaskStatusgainsEXPIRED. Unknown values already parsed asUNKNOWN, so code matching on statuses keeps working, but it may now seeEXPIRED.ResultURLUnavailable(aValueError) when a successful row's link stays missing.estimate_costprices1,"1","yes"and"on"at the base tier. The API rejects a job sending them.Testing
All of the following were run at the final head under
nice -n 19, one suite at a time.make test: 278 passed, 1 deselected (the live e2e test). New tests cover:verify_webhook:kidrotationnowand tolerance values, including with the age check offexpired, failed,pendingandprocessingrows, serially and withconcurrency=4ResultURLUnavailablewhen the second read still has no URL, and that it is aValueErrorexpiredparsing withurlstill anAnyUrlRun.triggerTaskSpendstill importing and parsing without a warningadd_tasks: with and without an idempotency key.pytest --cov=zenrows.batch --cov-report=term-missinggives 88% for the package.webhooks.pyis at 100%. No changed executable line insrc/zenrows/batch/is left uncovered; this was checked by intersecting the diff against coverage's missing lines.make check(ty, ruff check, ruff format --check): passes.make generateandmake docswere run, and their output is committed.Needs a human decision
errorthat lackstype,title,statusorcodestill fails validation of the whole results page, as on main. Making those fields optional would fix that, but it changes their types tostr | Noneand breaks strict type checking for current callers, so it is left out here.AnyUrlnormalises submitted URLs (https://Example.combecomeshttps://example.com/), so results can't always be matched to inputs by URL string. Fixing this without breaking callers would need a new field, such as a raw URL next tourl. It is left out here.feat, so the next release is a minor bump.