From d9f314a1784715c909400cfd775e9fb25e59e3fb Mon Sep 17 00:00:00 2001 From: Nikita Pinaev Date: Fri, 9 Oct 2026 15:33:39 +0300 Subject: [PATCH 1/2] STG-5405 Reconcile scan state after ambiguous start response --- docs/STG-5405.md | 41 ++++++++++ mdast_cli/ms_flow.py | 78 +++++++++++++++--- tests/test_smoke_flows.py | 21 +++++ tests/test_start_reconciliation.py | 123 +++++++++++++++++++++++++++++ 4 files changed, 250 insertions(+), 13 deletions(-) create mode 100644 docs/STG-5405.md create mode 100644 tests/test_start_reconciliation.py diff --git a/docs/STG-5405.md b/docs/STG-5405.md new file mode 100644 index 0000000..2d98fdf --- /dev/null +++ b/docs/STG-5405.md @@ -0,0 +1,41 @@ +# STG-5405: подтверждение запуска после потерянного ответа + +Задача: https://tracker.yandex.ru/STG-5405 + +## Проблема + +Scanyon продолжает запуск после таймаута Clark. Повтор Clark возвращает 409, +а CLI может получить ReadTimeout раньше ответа. Существующая обработка 409 +также принимает отсутствующую или отменённую стадию за успешный запуск. + +## Решение и границы + +- Отправлять POST запуска один раз. После Timeout/ConnectionError, HTTP 409 + или 502/503/504 проверять только GET существующего scan_id. +- Не более 6 запросов проверки с обычным интервалом опроса, без вложенных + циклов retry. Каждый HTTP-запрос сохраняет существующий timeout. +- Подтверждение: ответ с тем же ID, известной стадией START/WORKING/STOP/SUCCESS, + непустым строковым status, отличным от FAIL. Промежуточные статусы расширяются + сервером, поэтому их полный перечень не фиксируется в клиенте. +- FAIL/CANCELLED дают SCAN_FAILED, 401/403 сохраняют AUTH_ERROR. Created + проверяется повторно; неизвестные/неполные ответы не подтверждают запуск. +- Неопределённый результат содержит scan_id и указание проверить существующий + скан перед повторным запуском CLI. При 409 + Created сохраняется SCAN_FAILED. +- Штатный HTTP 200, ожидание скана, --nowait и монолит сохраняют поведение. + +Scanyon.start уже проверяет Created и fsm_locked под Redis-блокировкой FSM. +Его синхронный запуск и Clark в этой правке не меняются. Перенос в фоновую задачу +без гарантированного восстановления не входит в исправление CLI. + +## Приёмка + +Проверить потерянный ответ после принятия запуска, Created с последующим START, +исчерпание попыток, сетевые ошибки GET, HTTP 409/502/503/504, ошибки доступа, +FAIL/CANCELLED, пустой/неверный JSON и несовпадающий ID. Во всех сценариях +выполняется не более одного POST start и не создаётся дополнительный скан. +Проверить полный CLI flow с --nowait и обычным ожиданием, затем весь pytest. + +## Результат проверки + +Полный pytest: 443 passed, 1 skipped. Проверены оба CLI-сценария после +ReadTimeout: --nowait и ожидание результата, по одному созданию и запуску. diff --git a/mdast_cli/ms_flow.py b/mdast_cli/ms_flow.py index e121cf1..81d6d35 100644 --- a/mdast_cli/ms_flow.py +++ b/mdast_cli/ms_flow.py @@ -50,6 +50,7 @@ # upload path returns 429/502/503, worth a few retries, but not the full 5 min. UPLOAD_TRANSIENT_RETRIES = 3 UPLOAD_TRANSIENT_CODES = (429, 502, 503) +START_CONFIRM_ATTEMPTS = 6 # Strip C0/C1 control chars from server-controlled strings before logging, keeping # only TAB (\x09). CRUCIALLY this includes LF (\x0a) and CR (\x0d): otherwise a @@ -210,6 +211,69 @@ def _get_scan(mdast, scan_id): return _json_or_exit(resp, f'Getting scan info for scan {scan_id}') +def _confirm_scan_started(mdast, scan_id, conflict=False): + """Reconcile an ambiguous POST using bounded reads, never another POST.""" + action = f'Confirming start of scan {scan_id}' + last_stage = None + for attempt in range(START_CONFIRM_ATTEMPTS): + last_stage = None + try: + resp = mdast.get_scan_info(scan_id) + except (requests.Timeout, requests.ConnectionError) as exc: + logger.warning(f'{action}: {type(exc).__name__}') + else: + if resp.status_code in (401, 403): + _exit_on_http_error(resp, action) + if resp.status_code == 200: + try: + current = resp.json() + except ValueError: + current = None + if not isinstance(current, dict) or current.get('id') != scan_id: + break + last_stage = current.get('stage') + if last_stage in (ScanStage.FAIL, 'CANCELLED'): + logger.error(f'Scan {scan_id} failed or was cancelled: ' + f'{sanitize(current.get("message"))}') + sys.exit(ExitCode.SCAN_FAILED) + status = current.get('status') + if (last_stage in (ScanStage.START, ScanStage.WORKING, + ScanStage.STOP, ScanStage.SUCCESS) + and isinstance(status, str) and status.strip() + and status != ScanStageStatus.FAIL): + logger.info(f'Start confirmed for scan {scan_id}: ' + f'{sanitize(scan_pair(current))}') + return + if last_stage != ScanStage.CREATED: + break + elif resp.status_code not in POLL_TRANSIENT_CODES: + break + if attempt + 1 < START_CONFIRM_ATTEMPTS: + time.sleep(SLEEP_TIMEOUT) + logger.error(f'Cannot confirm start of scan {scan_id}; the request may have been accepted. ' + 'Check this existing scan before running the CLI again.') + sys.exit(ExitCode.SCAN_FAILED if conflict and last_stage == ScanStage.CREATED + else ExitCode.NETWORK_ERROR) + + +def _start_scan(mdast, scan_id): + """Submit once; a lost response does not imply that the scan did not start.""" + try: + resp = mdast.start_scan(scan_id) + except (requests.Timeout, requests.ConnectionError) as exc: + logger.warning(f'Start of scan {scan_id}: {type(exc).__name__}; checking scan state.') + _confirm_scan_started(mdast, scan_id) + return + if resp.status_code == 200: + return + if resp.status_code in (409, 502, 503, 504): + logger.warning(f'Start of scan {scan_id} returned HTTP {resp.status_code}; ' + 'checking scan state.') + _confirm_scan_started(mdast, scan_id, conflict=resp.status_code == 409) + return + _exit_on_http_error(resp, f'Starting scan {scan_id}') + + def run_precheck_gate(mdast, md5, profile_id, testcase_id, scan_type, os_version=None): """Gate model (DEC-671-05): any warning blocks the scan with a non-zero exit. @@ -414,19 +478,7 @@ def _run_microservices_flow(arguments, url, token, app_file, user_agent, verify) logger.info(f'Scan was created successfully. Scan id: {scan_id}') logger.info(f'Start scan with id {scan_id}') - start_resp = mdast.start_scan(scan_id) - if start_resp.status_code == 409: - # POST /scans/{id}/start/ is not idempotent: a facade retry (network/timeout - # after the first call already unlocked the scan) surfaces as 409 "not in - # initial state" even though the scan did start. Confirm by state before failing. - current = _get_scan(mdast, scan_id) - if current.get('stage') != ScanStage.CREATED: - logger.warning(f'Start returned 409 but scan {scan_id} is already past initial ' - f'state ({scan_pair(current)}) - treating as started (facade retry).') - else: - _exit_on_http_error(start_resp, f'Starting scan {scan_id}') - elif start_resp.status_code != 200: - _exit_on_http_error(start_resp, f'Starting scan {scan_id}') + _start_scan(mdast, scan_id) if arguments.nowait: logger.info('Scan successfully started. Don`t wait for end, exit with zero code') diff --git a/tests/test_smoke_flows.py b/tests/test_smoke_flows.py index df5d034..94632c0 100644 --- a/tests/test_smoke_flows.py +++ b/tests/test_smoke_flows.py @@ -10,6 +10,7 @@ from itertools import permutations import pytest +import requests import responses from tests.conftest import BASE_URL, REST_URL, TOKEN, application_json, run_main, scan_json @@ -340,3 +341,23 @@ def test_monolith_regression_flow(mocked_responses, monkeypatch, tmp_path, tmp_a if '/organizations/1/engines/' in c.request.url] assert engine_calls assert engine_calls[0].request.headers['Authorization'] == f'Token {TOKEN}' + + +@pytest.mark.parametrize('nowait', [True, False]) +def test_ms_lost_start_response_continues_existing_scan(mocked_responses, monkeypatch, + tmp_path, tmp_apk, apk_md5, + no_sleep, ms_mode, nowait): + monkeypatch.chdir(tmp_path) + register_ms_happy_path(mocked_responses, apk_md5) + mocked_responses.replace(responses.POST, f'{REST_URL}/scans/77/start/', + body=requests.ReadTimeout('response lost after acceptance')) + # Reconciliation reads WORKING; the normal flow subsequently reads SUCCESS. + args = ms_argv(tmp_apk) + (['--nowait'] if nowait else []) + assert run_main(monkeypatch, args) == 0 + calls = mocked_responses.calls + assert sum(call.request.method == 'POST' and call.request.url == f'{REST_URL}/scans/start/' + for call in calls) == 1 + assert sum(call.request.method == 'POST' and call.request.url == f'{REST_URL}/scans/77/start/' + for call in calls) == 1 + assert any(call.request.method == 'GET' and call.request.url == f'{REST_URL}/scans/77/' + for call in calls) diff --git a/tests/test_start_reconciliation.py b/tests/test_start_reconciliation.py new file mode 100644 index 0000000..16d3e5d --- /dev/null +++ b/tests/test_start_reconciliation.py @@ -0,0 +1,123 @@ +"""STG-5405: a lost start response must not create or start a second scan.""" +from unittest import mock + +import pytest +import requests + +from mdast_cli import ms_flow +from mdast_cli.helpers.exit_codes import ExitCode + +pytestmark = pytest.mark.unit + + +def response(code=200, payload=None): + return mock.Mock(status_code=code, json=mock.Mock(return_value=payload)) + + +def scan(stage='WORKING', status='WAITING', scan_id=77): + return response(payload={'id': scan_id, 'stage': stage, 'status': status}) + + +@pytest.mark.parametrize('result', [requests.ReadTimeout(), requests.ConnectionError(), + response(409), response(502), response(503), response(504)]) +def test_ambiguous_start_is_confirmed_without_another_post(result, no_sleep): + client = mock.Mock() + client.start_scan.side_effect = [result] + client.get_scan_info.side_effect = [scan('CREATED', 'INITIAL'), scan('START', 'RENT_ENGINE')] + ms_flow._start_scan(client, 77) + client.start_scan.assert_called_once_with(77) + client.create_auto_scan.assert_not_called() + client.create_manual_scan.assert_not_called() + assert client.get_scan_info.call_args_list == [mock.call(77), mock.call(77)] + + +def test_start_200_does_not_need_reconciliation(): + client = mock.Mock(start_scan=mock.Mock(return_value=response())) + ms_flow._start_scan(client, 77) + client.get_scan_info.assert_not_called() + + +@pytest.mark.parametrize('stage', ['START', 'WORKING', 'STOP', 'SUCCESS']) +def test_known_stages_confirm_start(stage): + client = mock.Mock(start_scan=mock.Mock(side_effect=requests.ReadTimeout()), + get_scan_info=mock.Mock(return_value=scan(stage))) + ms_flow._start_scan(client, 77) + client.start_scan.assert_called_once_with(77) + + +@pytest.mark.parametrize('payload', [None, [], {}, {'id': 77}, + {'id': 77, 'stage': 'UNKNOWN', 'status': 'INITIAL'}, + {'id': 77, 'stage': 'START'}, + {'id': 77, 'stage': 'START', 'status': ''}, + {'id': 77, 'stage': 'WORKING', 'status': 'FAIL'}, + {'id': 78, 'stage': 'WORKING', 'status': 'WAITING'}]) +def test_incomplete_or_unrelated_state_never_confirms_start(payload, caplog): + client = mock.Mock(start_scan=mock.Mock(return_value=response(409)), + get_scan_info=mock.Mock(return_value=response(payload=payload))) + with pytest.raises(SystemExit) as exc: + ms_flow._start_scan(client, 77) + assert exc.value.code == ExitCode.NETWORK_ERROR + assert 'Cannot confirm start of scan 77' in caplog.text + client.start_scan.assert_called_once_with(77) + + +@pytest.mark.parametrize('stage', ['FAIL', 'CANCELLED']) +def test_failed_or_cancelled_scan_is_not_success(stage): + client = mock.Mock(start_scan=mock.Mock(return_value=response(409)), + get_scan_info=mock.Mock(return_value=scan(stage))) + with pytest.raises(SystemExit) as exc: + ms_flow._start_scan(client, 77) + assert exc.value.code == ExitCode.SCAN_FAILED + + +@pytest.mark.parametrize('code', [401, 403]) +@pytest.mark.parametrize('during_get', [True, False]) +def test_auth_error_is_preserved(code, during_get): + client = mock.Mock(start_scan=mock.Mock(return_value=response(409 if during_get else code)), + get_scan_info=mock.Mock(return_value=response(code))) + with pytest.raises(SystemExit) as exc: + ms_flow._start_scan(client, 77) + assert exc.value.code == ExitCode.AUTH_ERROR + assert client.get_scan_info.call_count == int(during_get) + + +@pytest.mark.parametrize('conflict', [True, False]) +def test_created_is_bounded_and_keeps_scan_id(conflict, no_sleep, caplog): + client = mock.Mock(start_scan=mock.Mock(side_effect=[response(409) if conflict + else requests.ReadTimeout()]), + get_scan_info=mock.Mock(return_value=scan('CREATED', 'INITIAL'))) + with pytest.raises(SystemExit) as exc: + ms_flow._start_scan(client, 77) + assert exc.value.code == (ExitCode.SCAN_FAILED if conflict else ExitCode.NETWORK_ERROR) + assert client.get_scan_info.call_count == ms_flow.START_CONFIRM_ATTEMPTS + client.start_scan.assert_called_once_with(77) + assert 'Cannot confirm start of scan 77' in caplog.text + + +def test_transient_reads_share_single_retry_budget(no_sleep): + client = mock.Mock(start_scan=mock.Mock(side_effect=requests.ReadTimeout())) + client.get_scan_info.side_effect = [requests.ReadTimeout(), response(503), scan()] + ms_flow._start_scan(client, 77) + assert client.get_scan_info.call_count == 3 + client.start_scan.assert_called_once_with(77) + + +def test_read_network_errors_are_bounded(no_sleep, caplog): + client = mock.Mock(start_scan=mock.Mock(side_effect=requests.ReadTimeout()), + get_scan_info=mock.Mock(side_effect=requests.ReadTimeout())) + with pytest.raises(SystemExit) as exc: + ms_flow._start_scan(client, 77) + assert exc.value.code == ExitCode.NETWORK_ERROR + assert client.get_scan_info.call_count == ms_flow.START_CONFIRM_ATTEMPTS + assert 'Cannot confirm start of scan 77' in caplog.text + + +def test_non_json_state_does_not_confirm_start(caplog): + bad = response() + bad.json.side_effect = ValueError('HTML gateway response') + client = mock.Mock(start_scan=mock.Mock(side_effect=requests.ReadTimeout()), + get_scan_info=mock.Mock(return_value=bad)) + with pytest.raises(SystemExit) as exc: + ms_flow._start_scan(client, 77) + assert exc.value.code == ExitCode.NETWORK_ERROR + assert 'Cannot confirm start of scan 77' in caplog.text From d96d906540a0b2cae65a490d2b44bad3d205fb6e Mon Sep 17 00:00:00 2001 From: Nikita Pinaev Date: Fri, 9 Oct 2026 15:48:53 +0300 Subject: [PATCH 2/2] STG-5405 Prepare CLI release 2026.10.5 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 699a2b9..997d1af 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ setup( name="mdast_cli", - version='2026.10.4', + version='2026.10.5', python_requires='>=3.12',