Repository navigation
Conversation
…rrelation streams Align fcli aviator ssc audit-dast and correlate-sast-dast with the SAST audit client: keep at most 10 requests in flight per stream, retry SERVER_BUSY with backoff, fail individual items instead of the whole correlation run on per-request errors, always close the correlation stream, and use the same 500-minute overall timeout as SAST audit. Fixes 'DAST audit error: Internal server error' on large DAST FPRs.
ankit2995
changed the title
| this.streamId = UUID.randomUUID().toString(); | ||
| this.requestIssueIds.clear(); | ||
| this.sendWindow.clear(); | ||
| this.serverBusyAttempts.clear(); |
There was a problem hiding this comment.
nit (non-blocking): startStream resets sendWindow, serverBusyAttempts and requestIssueIds, but not requestMessages. CorrelationStreamProcessor clears all of them on restart (it clears requestMessages and serverBusyAttempts together). The generation guard prevents wrong resends, so this is harmless, but clearing requestMessages here too would keep the two processors consistent and avoid stale entries.
| } | ||
|
|
||
| /** Exponential delay for the given 1-based attempt, capped at {@code MAX_DELAY_MS}, plus jitter. */ | ||
| static long delayMs(int attempt) { |
There was a problem hiding this comment.
Optional, for a later phase (non-blocking): this backoff formula (base delay, exponential growth, cap, jitter) mirrors the one in AviatorStreamProcessor. Keeping it separate is the right call for this parity phase so the proven SAST path stays untouched. Once SAST is in scope (phase 3), consider consolidating into one shared implementation so the two do not drift.
| LOG.warn("Validation request {} failed: {} — {}", requestId, resp.getStatus(), resp.getStatusMessage()); | ||
| logger.progress("Validating " + received + " of " + state.totalValidationRequests + " correlation candidates"); | ||
| onValidationRequestFinished(received); | ||
| return; |
There was a problem hiding this comment.
Validation failures are counted here but do not affect the final status. Could we include them and report PARTIALLY_CORRELATED?
There was a problem hiding this comment.
Good point, done in e46aee1. Failed validations are now included in CorrelationResult and make the outcome PARTIALLY_CORRELATED, the same as failed or skipped correlation requests. They're still not saved as rejected pairs, so the next run retries them.
Resolve correlation imports and adapt stream tests to upstream FPR packages and execution contexts.
Summary
A customer's fcli aviator ssc audit-dast run failed with DAST audit error: Internal server error partway through a large DAST FPR. This change gives the DAST audit and correlation clients the same flow control the SAST audit client has long used in production. The customer's FPR now completes in full against an unchanged server.
Root cause
Each client controls how many requests it keeps outstanding on a stream. The SAST client caps this at a fixed in-flight limit. The DAST audit and correlation clients, which are newer, send every request as soon as the stream opens and rely on the server's gRPC flow control.
The server grants new gRPC flow-control credit as responses complete, so it doesn't cap the total outstanding work. On a large run the DAST stream built up several hundred outstanding requests on one pod.
That filled the per-tenant processing queue. The server then returned INTERNAL_ERROR and closed the stream.
SAST talks to the same server but never reaches this point, because its client never exceeds its in-flight limit.
Fix (client only, no protocol or server change)
DAST audit and correlation now use the same in-flight limit as SAST (Constants.INITIAL_REQUEST_WINDOW). A slot is freed only when its request gets a final response.
If the server replies SERVER_BUSY, the request is retried with backoff, up to the same limit SAST uses, and then marked failed on its own instead of failing the whole run.
Correlation: an error for one request now fails only that item, and a failed validation isn't saved as a rejected pair, so the next run retries it. The stream is always closed on exit.
The overall timeout is now 500 minutes for both DAST audit and correlation, the same as SAST. It was 300 seconds, which large runs exceed once requests are rate-limited.
Follow-up (separate, not blocking)
Server-side hardening so the server enforces these limits itself, which also protects older clients: bounded flow-control credit, and reporting a rejected request as SERVER_BUSY instead of ending the stream. A ticket will be raised.
Squash Message