Skip to content

sdkharness: observe real concurrency in large-transfer checks; harden resilience executable - #1182

Open
sophiecarreras wants to merge 3 commits into
masterfrom
sdkharness/observe-concurrency-harden-resilience
Open

sophiecarreras wants to merge 3 commits into
masterfrom
sdkharness/observe-concurrency-harden-resilience

Conversation

@sophiecarreras

Copy link
Copy Markdown
Contributor

What

Fixes review findings in the repository-owned .sdkharness/ checks.

  1. large.concurrent_parts and large.parallel_download now observe real concurrency instead of only comparing sizes and bytes. The conformance run has no control/journal listener, so the check points the CLI at a small loopback proxy (.sdkharness/tests/lib/observing_proxy.py) in front of the simulator. The proxy counts requests in flight and holds each part upload / download stream for 0.5 s (a client-side throttle) so overlap is visible.
    • concurrent_parts: --threads 1 must peak at exactly 1 b2_upload_part in flight, --threads 4 at 2 or more; the uploaded bytes are now also downloaded and compared.
    • parallel_download: --threads 1 must use one stream and no Range request; --threads 4 must make at least one ranged GET and have two or more download streams overlapping. b2sdk only splits a download per 100 MiB of content and the CLI cannot lower that, so the fixture is just over 200 MiB.
    • Mutation check: forcing --threads 1 in the check makes both fail with a concurrency message.
  2. resilience-upload.py: follows LATEST_STABLE_VERSION (no hard-coded b2v5) and asserts b2 is imported from this checkout; contentSha1: none is accepted only for the large-file scenario (and then fileInfo.large_file_sha1 must match); the scenario field is filled in the result line when the configuration is refused (unknown if it is not a known scenario); the first account authorize setup step is retried up to 3 times with a NOTE (the earlier transient authenticate: b2 exited 1 on a cold first run could not be reproduced; authorize is idempotent and not under test, so a persistent failure still fails).
  3. Proxy variables (HTTP(S)_PROXY, ALL_PROXY, NO_PROXY, either case) are scrubbed from the CLI child environment in all four executables via simulator_guard.scrubbed_environment (loopback is exempted explicitly), and the printed-URL fetch in urls.native_download bypasses ambient proxies.
  4. New unit tests, test/unit/test_sdkharness_guards.py, for the loopback URL guard, the fixed-credential guard (no echo of the refused key), scrubbing, the resilience helpers and the observing proxy.
  5. Changelog: +sdkharness-observe-concurrency.infrastructure.md for this change, and +sdkharness-migration.infrastructure.md recording the migration PRs test: add repository-owned bucket conformance contracts #1171-Complete repository-owned CLI resilience coverage #1177, which were merged without individual fragments (CONTRIBUTING requires one per PR).

Not changed on purpose: upload.retry_408 and api.retry_after_503 are real known CLI findings and still fail.

Testing

Against a loopback simulator only, with HTTP_PROXY/HTTPS_PROXY set to a dead port in the environment:

  • conformance: 30/30 PASS (the two large scenarios 4 consecutive runs each, stable), health golden-path PASS.
  • resilience: 14 PASS, upload.stall SKIP (no-client-option), upload.retry_408 and api.retry_after_503 FAIL (the known findings), as before.
  • pytest test/unit/test_sdkharness_guards.py: 94 passed; the tests fail when the guards are removed. test/static and ruff check pass.

🤖 Generated with Claude Code

@sophiecarreras sophiecarreras self-assigned this Oct 4, 2026
sophiecarreras and others added 3 commits October 15, 2026 16:15
…ilience executable

- large.concurrent_parts and large.parallel_download now observe overlapping
  b2_upload_part requests and download streams (ranged GETs) through a loopback
  observing proxy with a client-side hold, and verify the uploaded bytes.
- resilience-upload.py follows LATEST_STABLE_VERSION and asserts it runs from
  this checkout, accepts contentSha1 'none' only for large files, names the
  scenario in the result line on configuration failure, and retries only its
  own authorize setup step.
- Proxy variables are scrubbed from every CLI child environment and the
  printed-URL fetch bypasses ambient proxies.
- Unit tests for the loopback and fixed-credential guards.
- Changelog fragments, including the migration PRs #1171-#1177.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…serving proxy

- Update test_sdkharness_resilience for the stricter contentSha1 rule and add
  a large-file positive case.
- observing_proxy: forward only origin-form paths, validate Content-Length,
  stream non-JSON upstream bodies instead of buffering them.
- Merge the two changelog fragments into one; rewrap the README paragraph.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sophiecarreras
sophiecarreras force-pushed the sdkharness/observe-concurrency-harden-resilience branch from 30a259d to 685dc49 Compare October 15, 2026 19:16
@sophiecarreras

Copy link
Copy Markdown
Contributor Author

@ffumero2003 this PR is ready for review. Every GitHub Actions job is green after the rebase onto master (the earlier red runs came from master's SSE-B2 integration tests before #1185). The only red check is code/snyk, which reports 2 new Code Analysis issues. We cannot see the findings without Snyk access. By reading the added code, the likely candidates are test-only: dummy key-shaped values in test/unit/test_sdkharness_guards.py, SHA-1 used for B2's integrity checksums in the resilience test, and the local test proxy that connects to a fixed 127.0.0.1 upstream. Could you open the Snyk check, confirm which two it flags, and dismiss them as test code if they are those? Thank you.

@ffumero2003

Copy link
Copy Markdown
Contributor

Reviewed the diff locally (the observing proxy, the concurrency assertions, and the resilience/guard changes) — looks good, approving. The two code/snyk findings are the bare hashlib.sha1(payload) calls at .sdkharness/tests/resilience-upload.py:336 and :470, both false positives: SHA-1 only verifies B2’s required contentSha1/large_file_sha1 content checksum, not a security primitive. The credential constants are rejected test fixtures and the observing proxy is loopback-only (no SSRF/public bind). Tests pass 121/121, test/static and Ruff pass; no correctness blockers. Suggest adding usedforsecurity=False to those two calls (matching conformance-file-lifecycle.py:187) to make intent explicit and clear the informational Snyk check — code/snyk isn’t required, so not merge-blocking either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants