fix(bitdefender): fourth-pass - urllib reset safety, Retry-After clamp, sweep/install-links id validation
From a third review pass (converging - all MEDIUM/LOW): - urllib fallback: a post-send reset (RemoteDisconnected/ConnectionReset, which urllib wraps in URLError) was misclassified as always-safe 'connect' and could retry a non-idempotent write after a server commit. Now only ConnectionRefused/ DNS (socket.gaierror) -> 'connect'; everything else -> 'timeout' (write-gated). - _retry_delay clamps a negative numeric Retry-After to 0 (was -> time.sleep(-1) ValueError). - cmd_sweep + cmd_install_links now validate --company; cmd_company_create validates --parent (finished _require_oid consistency - these mislogged as errorlog noise). - cmd_push_test parses --extra-json before gating (validate->gate order, matches siblings). - selftest: +sweep/install-links bad-company assertions. 81/81. Units: clamp + reset classification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -284,6 +284,8 @@ def cmd_company_create(client, args):
|
|||||||
extra, rc = _load_json_arg(args.extra_json, "extra-json")
|
extra, rc = _load_json_arg(args.extra_json, "extra-json")
|
||||||
if rc:
|
if rc:
|
||||||
return rc
|
return rc
|
||||||
|
if args.parent and not _require_oid(args.parent, "parent company"):
|
||||||
|
return 2
|
||||||
label = {0: "Partner", 1: "Customer"}.get(args.type, str(args.type))
|
label = {0: "Partner", 1: "Customer"}.get(args.type, str(args.type))
|
||||||
if not _gated(f"create {label} company '{args.name}'", args.confirm):
|
if not _gated(f"create {label} company '{args.name}'", args.confirm):
|
||||||
return 3
|
return 3
|
||||||
@@ -337,6 +339,8 @@ def cmd_endpoint(client, args):
|
|||||||
|
|
||||||
|
|
||||||
def cmd_sweep(client, args):
|
def cmd_sweep(client, args):
|
||||||
|
if args.company and not _require_oid(args.company, "company"):
|
||||||
|
return 2
|
||||||
if args.company:
|
if args.company:
|
||||||
summaries = client.security_sweep(args.company)
|
summaries = client.security_sweep(args.company)
|
||||||
else:
|
else:
|
||||||
@@ -535,11 +539,11 @@ def cmd_push_set(client, args):
|
|||||||
|
|
||||||
|
|
||||||
def cmd_push_test(client, args):
|
def cmd_push_test(client, args):
|
||||||
if not _gated(f"send test push event '{args.event_type}'", args.confirm):
|
|
||||||
return 3
|
|
||||||
extra, rc = _load_json_arg(args.extra_json, "extra-json")
|
extra, rc = _load_json_arg(args.extra_json, "extra-json")
|
||||||
if rc:
|
if rc:
|
||||||
return rc
|
return rc
|
||||||
|
if not _gated(f"send test push event '{args.event_type}'", args.confirm):
|
||||||
|
return 3
|
||||||
result = client.send_test_push_event(args.event_type, extra=extra or None)
|
result = client.send_test_push_event(args.event_type, extra=extra or None)
|
||||||
_emit({"testEvent": args.event_type, "result": result}, args.json, _print_kv)
|
_emit({"testEvent": args.event_type, "result": result}, args.json, _print_kv)
|
||||||
return 0
|
return 0
|
||||||
@@ -702,8 +706,11 @@ def cmd_create_package(client, args):
|
|||||||
|
|
||||||
|
|
||||||
def cmd_install_links(client, args):
|
def cmd_install_links(client, args):
|
||||||
|
if args.company and not _require_oid(args.company, "company"):
|
||||||
|
return 2
|
||||||
_emit(client.get_installation_links(args.package, args.company),
|
_emit(client.get_installation_links(args.package, args.company),
|
||||||
args.json, _print_kv)
|
args.json, _print_kv)
|
||||||
|
return 0
|
||||||
|
|
||||||
|
|
||||||
def cmd_scan(client, args):
|
def cmd_scan(client, args):
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import base64
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import random
|
import random
|
||||||
|
import socket
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
@@ -86,7 +87,9 @@ def _retry_delay(headers, attempt: int) -> float:
|
|||||||
# An explicit server-mandated Retry-After is honored up to a HIGHER cap
|
# An explicit server-mandated Retry-After is honored up to a HIGHER cap
|
||||||
# than the exponential backoff (don't retry early into another 429).
|
# than the exponential backoff (don't retry early into another 429).
|
||||||
try:
|
try:
|
||||||
return min(float(ra), RETRY_AFTER_MAX_SECONDS)
|
# clamp to [0, ceiling]: a malformed negative Retry-After must not
|
||||||
|
# reach time.sleep() (which raises ValueError on a negative value).
|
||||||
|
return min(max(float(ra), 0.0), RETRY_AFTER_MAX_SECONDS)
|
||||||
except (TypeError, ValueError):
|
except (TypeError, ValueError):
|
||||||
try:
|
try:
|
||||||
dt = parsedate_to_datetime(ra)
|
dt = parsedate_to_datetime(ra)
|
||||||
@@ -373,11 +376,16 @@ class GravityZoneClient:
|
|||||||
except TimeoutError as exc:
|
except TimeoutError as exc:
|
||||||
raise _RetryableHTTP("timeout", detail=str(exc)) from exc
|
raise _RetryableHTTP("timeout", detail=str(exc)) from exc
|
||||||
except urllib.error.URLError as exc:
|
except urllib.error.URLError as exc:
|
||||||
# A bare URLError (not HTTPError) is a connection-level failure. If it
|
# Classify conservatively: only a KNOWN pre-send failure (connection
|
||||||
# wraps a read timeout it is ambiguous ('timeout'); otherwise it is a
|
# refused / DNS failure) is the always-safe 'connect'. Anything else -
|
||||||
# pre-send connect failure ('connect', always safe to retry).
|
# a connect/read timeout, or an ambiguous post-send reset like
|
||||||
|
# RemoteDisconnected/ConnectionResetError that urllib also wraps in
|
||||||
|
# URLError - is 'timeout' so a non-idempotent write is NOT retried.
|
||||||
reason = getattr(exc, "reason", None)
|
reason = getattr(exc, "reason", None)
|
||||||
code = "timeout" if isinstance(reason, TimeoutError) else "connect"
|
if isinstance(reason, (ConnectionRefusedError, socket.gaierror)):
|
||||||
|
code = "connect"
|
||||||
|
else:
|
||||||
|
code = "timeout"
|
||||||
raise _RetryableHTTP(code, detail=str(exc)) from exc
|
raise _RetryableHTTP(code, detail=str(exc)) from exc
|
||||||
try:
|
try:
|
||||||
return json.loads(raw.decode("utf-8"))
|
return json.loads(raw.decode("utf-8"))
|
||||||
|
|||||||
@@ -95,6 +95,8 @@ check("policy no shallow warning", ["policy", "5c42940b6e16d61a0c8b4568"], want_
|
|||||||
# behavior, not a skill bug. ---
|
# behavior, not a skill bug. ---
|
||||||
check("endpoint bad id -> rc2 (client-side)", ["endpoint", "bogus"], want_rc=2, err_has="not a valid")
|
check("endpoint bad id -> rc2 (client-side)", ["endpoint", "bogus"], want_rc=2, err_has="not a valid")
|
||||||
check("policy bad id -> rc2 (client-side)", ["policy", "bogus"], want_rc=2, err_has="not a valid")
|
check("policy bad id -> rc2 (client-side)", ["policy", "bogus"], want_rc=2, err_has="not a valid")
|
||||||
|
check("sweep bad company -> rc2 (client-side)", ["sweep", "--company", "bogus"], want_rc=2, err_has="not a valid")
|
||||||
|
check("install-links bad company -> rc2", ["install-links", "--package", "P", "--company", "bogus"], want_rc=2, err_has="not a valid")
|
||||||
|
|
||||||
# --- argparse: missing required arg -> rc2 ---
|
# --- argparse: missing required arg -> rc2 ---
|
||||||
check("quarantine missing --company -> rc2", ["quarantine"], want_rc=2)
|
check("quarantine missing --company -> rc2", ["quarantine"], want_rc=2)
|
||||||
|
|||||||
Reference in New Issue
Block a user