Conversation
…d-scans feat(api): auto-retry failed pentest scans and clean up failure messages
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
* fix(api): pentest retry follow-ups from production review - Enforce one auto-retry child per parent (unique index on retry_of_provider_run_id, dedup-guarded migration) so concurrent duplicate failure webhooks can't create two retry scans or a nondeterministic active attempt. - Preserve an explicit empty check selection ([]) on retry instead of dropping it (which let the provider fall back to its default check set). - Only mask a failure as in-progress when the run is actually retry-eligible (under the attempt cap and has stored scan params), so pre-feature runs with no scan params reveal their failure immediately instead of showing provisioning for the grace window. - Treat a malformed non-positive runtime cap (e.g. "cap 0m") as unrecognized and return the generic message instead of "maximum runtime of its time limit". * fix(api): dedupe pentest retries at the provider via idempotency key Replaces the DB unique-constraint approach with a Maced idempotency key so concurrent duplicate failure webhooks dedupe at the provider — the key fix, since the constraint only prevented duplicate rows AFTER both requests had already launched a provider scan (orphaning the loser). - Auto-retries pass a deterministic Idempotency-Key (`retry:<parentRunId>`); Maced returns the same run for concurrent duplicates, so only one scan runs and only one ownership row is written (via the existing providerRunId upsert). - Drop the retry_of_provider_run_id unique index + its dedup migration (no longer needed, and the migration's row-delete could orphan provider runs). - Retry-eligibility (for status masking) now reuses fromScanParams validation instead of a shallow null check, so malformed stored params reveal the failure immediately rather than masking it. - Runtime-cap message now requires a safe integer cap, so absurd oversized caps fall through to the generic message. * fix(api): make pentest retry billing reservation idempotent Concurrent duplicate failure webhooks converged on one provider run (via the idempotency key) but still reserved two subscription allowances — each call used a random reservation id, so only one landed on the shared ownership row and the other allowance was orphaned (never refunded), double-charging the customer. Retries now use a deterministic reservation id (`pending:retry:<parentRunId>`); billing consumption is idempotent on it, so duplicates debit exactly once.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…redelivery) (#3401) The deterministic pending:retry:<parent> reservation id introduced a worse, realistic bug: billing consume is idempotent on the id AND a refund leaves the consume event in place, so a failed-create-then-refund followed by the standard webhook redelivery would reuse the key, skip the debit, and run the scan for free. Revert to a unique random reservation id so redelivery re-debits correctly. Concurrent duplicate SCANS are still deduped by the Maced idempotency key; the residual concurrent-only over-charge requires concurrent duplicate webhook delivery (unverified) and is not worth the regression risk of a shared reservation.
Contributor
|
🎉 This PR is included in version 3.101.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.
Summary by cubic
Automatically retries failed penetration-test runs, dedupes duplicate webhook‑triggered retries at the provider, and collapses each lineage into a single, stable run in the UI. Cleans up failure messages and keeps status/progress consistent to hide transient failures.
New Features
checkslist on retry; small UI tweak: remove the “Common causes” list and show the refined failure message.Migration
Written for commit 6033674. Summary will update on new commits.