Skip to content

Add nineteen sdkharness conformance checks: large files, copy, URLs, locks, encryption, keys, bucket configuration - #222

Open
sophiecarreras wants to merge 2 commits into
masterfrom
sdkharness/conformance-slice-2
Open

sophiecarreras wants to merge 2 commits into
masterfrom
sdkharness/conformance-slice-2

Conversation

@sophiecarreras

@sophiecarreras sophiecarreras commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Context

Adds 19 conformance scenarios to the SDK harness, bringing the total to 27 alongside the existing health check and 16 resilience scenarios.

Changes

Adds coverage for:

  • Server-side copy.
  • Multipart uploads, concurrent parts, resume, and mixed uploaded/copied parts.
  • Native download URLs and authorization.
  • Upload progress callbacks.
  • Object Lock defaults, per-file retention, legal hold, and governance bypass.
  • SSE-B2 and SSE-C encryption.
  • Application key creation, listing, authorization, and revocation.
  • Bucket CORS, lifecycle, notification rules, inbound webhook verification, and replication configuration.

Shared helpers provide loopback guards, request recording, concurrency measurement, and cleanup of created resources. Both conformance and resilience runners explicitly compile sources as UTF-8.

Only .sdkharness/ changes; SDK implementation and Gradle configuration remain unchanged.

Verification

At commit 12af861:

  • All conformance sources compile.
  • GuardSelfTest passes.
  • 26 of 27 conformance scenarios pass.
  • large.resume correctly reports FAIL for an existing SDK bug: resuming with the original request is rejected when the content source provides a known SHA-1. The check preserves the original request and does not work around the failure.
  • Key revocation is checked through fresh account authorization.

The reviewer independently reproduced these results using a local simulator revision different from the originally cited pin.

GitHub build, dependency-security, and license checks pass. Snyk Code still reports two issues requiring investigation.

…locks, encryption, keys, bucket configuration

Slices 4 and 5 of the Java conformance plan, completing one check for each of the 27 capabilities this SDK
claims: files.server_side_copy, large.multipart, large.concurrent_parts, large.resume, large.compose_mixed,
urls.native_download, client.progress, lock.bucket_default, lock.per_file_retention, lock.legal_hold,
lock.bypass_governance, enc.sse_b2, enc.sse_c, keys.crud, bucket.cors, bucket.lifecycle,
bucket.notification_rules, bucket.inbound_webhook, bucket.replication_config.

Support.java gains an executor helper, an in-flight request counter on the recording client, Object Lock and
key cleanup, a second client for a key the check created, and a loopback-only raw GET for share links
(covered by GuardSelfTest). The README table lists the new scenarios.

Both run-conformance and run-resilience now pass -encoding UTF-8 to javac, so a check source with a
non-ASCII literal compiles under LC_ALL=C on a JDK before 18 (the harness's daily run exports LC_ALL=C).

@ffumero2003 ffumero2003 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — it's a big, careful slice, and most of it is exactly what we want: the checks exercise the public API, the concurrency measurement in LargeConcurrentParts is a genuine in-flight count off the recording client (not inferred from wall time), and Support.cleanup() is thorough on the failure path (cancels unfinished large files, releases legal holds and bypasses governance before deleting locked versions, deletes tracked keys, shuts down executors). Requesting changes on two correctness issues plus the red Snyk check before it lands.

Blocking

1. LargeResume turns a real SDK failure into a PASS (masks a bug)

In LargeResume.java (the resume leg, ~L91–104), the check first calls finishUploadingLargeFile(...) with the original request (the large-file-sha1 custom field left null). When the SDK rejects it with a "mismatch" B2LocalException, the check catches it, logs a FINDING ... note, and then re-runs finishUploadingLargeFile with a different request — one that re-supplies sha1 as the custom field — and reports PASS.

That's the problem: the scenario detects that the SDK refuses to resume the same large file with the same request that started it, and then works around that refusal so the cell still reads PASS. Our harness is detect-only — when a scenario hits genuine broken behavior it must report FAIL so the dashboard surfaces the gap, not paper over it with a modified request. As written, large.resume is green while the behavior it exists to prove (resume an interrupted large file with the request you started it with) is actually broken.

Please keep the original request and let the scenario FAIL (surfacing the FINDING text as the failure reason), and track the underlying finishUploadingLargeFile behavior as a separate SDK bug/fix. The workaround branch (the retry that passes sha1 into the custom field) should come out.

2. KeysCrud deleted-key assertion also passes for a valid key

In KeysCrud.java (~L83–84), revocation is asserted by expecting HTTP 401 from:

Support.clientFor(created.getApplicationKeyId(), secret).buckets()

But created is a bucket-restricted key (earlier in the check it's verified to list only its one bucket and its authorization is scoped to that bucket id). A bucket-restricted key calling .buckets() — i.e. b2_list_buckets with no bucket named — returns 401 even when the key is perfectly valid, because a restricted key isn't allowed to list all buckets. So this assertion passes whether or not the delete actually revoked anything; it can't establish credential revocation. (Reproduced with a live, un-deleted key: same 401.)

Please assert on authorization itself instead — e.g. call getAccountAuthorization() (a fresh b2_authorize_account) with the deleted key's id/secret and assert that authorization fails. That genuinely distinguishes a revoked key from a valid restricted one.

3. code/snyk is red — two new Code Analysis issues

CI's code/snyk reports 2 new issues; build, license/snyk, and security/snyk are green. I can't see the Snyk Code output directly, so please confirm the specifics there, but reading the diff the two likely hits are:

  • Support.rawGet — SSRF (openConnection() on a tainted URL). The method guards loopback (throws unless the host is 127.0.0.1, and GuardSelfTest now covers both the pass and block paths), so it's safe at runtime — but it parses the URL twice (URI.create(url) for the host check, then URI.create(url).toURL() for the connection), so static analysis can't see the value that reaches openConnection() as sanitized. Parsing once into a single URI/URL, validating getHost() on that object, and connecting on the same object should both clear Snyk and remove the double-parse smell. If it still flags, a scoped suppression with the justification "loopback-only harness code, host asserted 127.0.0.1, covered by GuardSelfTest" is fine.
  • A crypto/secret false positive — most likely the MD5 digest in EncSseC (the ...Customer-Key-Md5 header is mandated by the B2 SSE-C protocol; it's a protocol checksum of a throwaway test key, not a security hash), or the hardcoded SECRET = "..." literal in BucketInboundWebhook (an offline test fixture). Either is a false positive → keep the value and add a justified suppression with a one-line reason.

Non-blocking (optional, your call)

  • LargeConcurrentParts resets maxInFlight between the one-worker and four-worker legs but not the shared inFlight counter; if a request were ever sent without a response, the leftover increment could make the strict serialMax == 1 assertion flake. It won't against the fault-free simulator, but a Support.inFlight.set(0) beside the existing maxInFlight.set(0) would harden it.
  • Support.cleanup() deletes keys before buckets; for BucketReplicationConfig the source bucket references the source key id, so on a stricter backend the bucket delete could error (surfaces as a cleanup FAIL, never a silent leak). Fine on the simulator.
  • UrlsNativeDownload uses a wall-clock token-expiry margin (2s validity + 1.5s). Generous, acceptable.

Once 1 and 2 are addressed and code/snyk is green, this is good to go — the rest is solid.

- large.resume: drop the retry that re-supplied sha1 as the custom field; the original request is kept and the scenario fails with the FINDING as its reason when the SDK refuses it with "mismatch".
- keys.crud: assert revocation through a fresh authorization (getAccountAuthorization) with the deleted key; the earlier authorization of the live restricted key is the control.
- Support.rawGet: parse the URL once and validate and connect on the same URI.
- large.concurrent_parts: reset inFlight beside maxInFlight.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@sophiecarreras

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. Pushed 12af861 with the changes below.

  1. LargeResume: removed the retry that re-supplied sha1 as the custom field. The original request is kept, and when the SDK refuses it with the "mismatch" B2LocalException the scenario now fails with the FINDING text as the reason. Against the simulator large.resume is now FAIL (leg "sha1"; the leg without a known sha1 resumes fine). The README note is updated to match. We are not changing the SDK here; the underlying finishUploadingLargeFile behavior is a separate bug.
  2. KeysCrud: revocation is now asserted through authorization itself: getAccountAuthorization() on a fresh client with the deleted key's id and secret must fail with 401. The same key was authorized successfully (and its bucket scope checked) just before the delete, so the assertion tells a revoked key from a valid restricted one.
  3. code/snyk: Support.rawGet now parses the URL once into a single URI, checks getHost() on it (127.0.0.1 only) and connects with uri.toURL() on that same object. GuardSelfTest still passes. I have no access to the Snyk Code details, so I did not add any suppression for the possible MD5 (EncSseC) or SECRET (BucketInboundWebhook) hits; if the check stays red after this push, please tell me which two findings it lists and I will add a justified suppression in the form the repo uses.

Also applied the non-blocking one: Support.inFlight.set(0) beside maxInFlight.set(0) in LargeConcurrentParts.

All 27 checks were run against the simulator with this checkout: 26 pass, large.resume fails as described.

@ffumero2003
ffumero2003 dismissed their stale review October 23, 2026 17:11

Errors fixed, we can't fix the snyk, cause: we don't have access

This branch has not been deployed

No deployments
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