Skip to content

Add eight sdkharness conformance checks: file operations and bucket CRUD - #220

Merged
sophiecarreras merged 2 commits into
masterfrom
sdkharness/conformance-slice-1
Oct 14, 2026
Merged

sophiecarreras merged 2 commits into
masterfrom
sdkharness/conformance-slice-1

Conversation

@sophiecarreras

Copy link
Copy Markdown
Contributor

Summary

Adds eight conformance scenarios to the .sdkharness/ contract (health is unchanged), so Backblaze's SDK quality harness can ask whether this SDK really does what its capability card claims. This is the first slice: the core file and bucket operations.

  • .sdkharness/tests.tsv: eight rows, conformance <capability> simulator ./.sdkharness/tests/run-conformance, for files.upload, files.download_content, files.download_by_id, files.list, files.delete_version, files.hide, files.metadata and bucket.crud.
  • .sdkharness/tests/run-conformance (bash, executable): same guards as run-health (loopback simulator URL only, every ambient B2_* dropped, fixed test credential). It compiles conformance/Support.java and the scenario's class with javac --release 11 against the jars the harness built, runs it, and prints one SDKHARNESS_RESULT line. A JDK 11 single-file launch cannot import a second source file, which is why the shared helper is compiled rather than imported.
  • .sdkharness/tests/conformance/*.java: one program per scenario, using only the SDK's public API. Each one cites the B2 API pages its assertions come from, makes its own sdkharness-conf-* buckets, and deletes everything it created, on failure too.
  • .sdkharness/README.md: what each scenario asserts and how to run one by hand.

files.metadata is partial on the card (by name is emulated with an HTTP HEAD of the download URL). The check records the SDK's own HTTP requests and asserts that boundary: by id is a POST to b2_get_file_info, by name is a HEAD of /file/<bucket>/<name>.

No build or source changes. The Gradle files, core/ and httpclient/ are untouched, and .sdkharness/ is in no source set. There is no test in this repository that pins the contract file.

Verification

Run through the harness adapter against the pinned standalone simulator (backblaze-labs/b2-simulator at 21a4002), with the jars the harness built from this branch's revision, under a JDK 17 launcher (and the wrapper by hand under JDK 11 for files.list, files.metadata, bucket.crud):

Scenario Result
files.download_content, files.download_by_id, files.list, files.delete_version, files.hide, bucket.crud, files.metadata PASS
files.upload FAIL, unusual name: a file named st/upload/sp ace/... comes back as st/upload/sp+ace/...

The files.upload failure is a finding, not a loosened check. B2 documents X-Bz-File-Name as "percent-encoded UTF-8. For example, spaces should be replaced with %20" (b2_upload_file). B2StringUtil.urlEncode is URLEncoder.encode(s, UTF8).replace("%2F", "/"), which writes a space as +, so the stored name contains a literal plus. The other legs of files.upload (size, sha1, content type, fileInfo, listing, byte-identical download, empty file) pass before that leg runs. Whether to change the encoding is the maintainers' call; the check will go green when it does.

./gradlew build is unaffected by this change.

Merge note

A merge to master runs the existing ci_cd.yml push steps (upload to b2, Javadoc to gh-pages); this PR does not change them.

Tracking: backblaze-labs/demand-side-ai#1101

@ffumero2003

Copy link
Copy Markdown

Reviewed scope, the security guards, and each scenario. Well-scoped — only .sdkharness/ changes, nothing in a Gradle source set, so none of this reaches the published artifact, and the eight checks use only the public API and match the B2 docs they cite. Three things before I approve, mostly so we're not merging on a red check and so the README's guarantees actually hold.

Required (blocking)

1. Snyk Code is red (1 new finding) — needs to be green before merge.
The likely candidate is the SHA-1 in Support.java:264. If so it's a false positive — B2 requires SHA-1 as the upload integrity checksum (X-Bz-Content-Sha1), it's not a security primitive, and this is test-only code. Please grab the exact rule id from the Snyk check and add a scoped, documented ignore so the check passes, e.g. a .snyk policy:

# .snyk
version: v1.5.0
ignore:
  '<SNYK-RULE-ID-FROM-THE-CHECK>':
    - '.sdkharness/tests/conformance/Support.java':
        reason: >-
          SHA-1 is B2's required upload integrity checksum (X-Bz-Content-Sha1),
          not a cryptographic/security primitive. Test-only; not in any source set.
        expires: 2027-10-13T00:00:00.000Z

(Or ignore it in the Snyk UI with the same reason — either way, confirm the rule/line first in case it's not the SHA-1 one.)

2. A failed cleanup must fail the check. Right now cleanup() (Support.java:141) only prints a NOTE on a delete error, and run() (:103) still prints PASS — which contradicts the README's "deletes everything … on failure." Collect cleanup errors and fail the verdict if any occurred:

private static final List<String> cleanupErrors = new ArrayList<>();

private static void cleanup() {
    if (client == null) return;
    for (B2Bucket bucket : created) {
        try {
            client.deleteAllFilesInBucket(bucket.getBucketId());
            client.deleteBucket(bucket.getBucketId());
        } catch (B2Exception | RuntimeException e) {
            cleanupErrors.add(bucket.getBucketName() + ": " + describe(e));
        }
    }
    try { client.close(); } catch (RuntimeException e) { /* nothing left to release */ }
}

…and in run(), after the finally:

if (failure == null && !cleanupErrors.isEmpty()) {
    failure = "cleanup -- left " + cleanupErrors.size() + " bucket(s): "
            + tidy(String.join("; ", cleanupErrors));
}

That attempts every deletion and still turns a leaked bucket into a FAIL.

3. Enforce the loopback boundary at request time (or soften the claim). The guard at Support.java:89 only validates the configured authorize origin; the SDK then follows api/download/upload URLs from responses, which aren't constrained — so the comment "the SDK's http switch must never reach anything else" isn't actually guaranteed. Use one guarded factory for every check (not just the recording path) that rejects any non-loopback target:

private static HttpClientFactory guardedFactory(RequestConfig requestConfig, boolean recordWire) {
    return new HttpClientFactory() {
        @Override public CloseableHttpClient create() {
            return HttpClients.custom()
                .setDefaultRequestConfig(requestConfig)
                .addInterceptorFirst((HttpRequestInterceptor) (request, context) -> {
                    final HttpHost target = (HttpHost) context.getAttribute(
                        HttpCoreContext.HTTP_TARGET_HOST);   // confirm this is populated pre-send
                    final String host = target == null ? null : target.getHostName();
                    if (!"127.0.0.1".equals(host)) {
                        throw new HttpException("blocked non-loopback host: " + host);
                    }
                    if (recordWire) {
                        wire.add(request.getRequestLine().getMethod() + " " + request.getRequestLine().getUri());
                    }
                })
                .build();
        }
        @Override public void close() {}
    };
}

and have newClient always use guardedFactory(...). One caveat to verify: confirm HTTP_TARGET_HOST is set on the context at addInterceptorFirst time for redirects too; if not, read the host from the request URI / request line instead. If you'd rather not enforce it, the alternative is to drop the absolute wording and state only what's guaranteed (the configured origin is loopback; ambient B2_* are dropped).

Recommended (non-blocking)

  • Anchor the verdict match. run-conformance:55 uses grep -F, which matches the marker anywhere in a line (run-health anchors it). Anchor at column 1 — using index() avoids the . in scenario names being treated as regex:

    # non-result lines
    awk -v p="$prefix" 'index($0,p)!=1' "$log" || true
    # count
    count="$(awk -v p="$prefix" 'index($0,p)==1{n++} END{print n+0}' "$log")"
    # verdict line
    verdict="$(awk -v p="$prefix" 'index($0,p)==1' "$log")"
  • Soften the redaction wording. tidy()/describe() format and cap output but aren't redaction, and other log lines are forwarded unchanged (run-conformance:56). Fixed sim credentials limit exposure, so it's fine in practice — just narrow the "no response body or secret can be printed" claim, or allowlist the diagnostic fields.

Keep as-is

The files.upload FAIL is a legitimate SDK bug, not a bad test — keep it. B2 requires a space in X-Bz-File-Name to be %20, but B2StringUtil.urlEncode uses form-style URLEncoder, which writes a space as +; the literal + later in the test name is encoded separately as %2B, so it doesn't confound the finding. Fair, deterministic result — whether to change the encoding is the maintainers' call.

Verdict

Request changes — happy to approve once Snyk is green, a failed cleanup fails the check, and the loopback boundary is enforced (or the claim scoped). Everything else is solid.

…ime, anchor the verdict match

- A bucket or file the check could not delete now fails the verdict; every deletion is still attempted and the single result line is still printed.
- Every check builds its client from one guarded HTTP client factory that refuses any request whose target host is not 127.0.0.1 (redirect hops included) and fails the verdict if one was attempted. GuardSelfTest proves it.
- run-conformance matches the result line at column 1 with index(), not anywhere in a line.
- README: say what is guaranteed (formatting and capping, not redaction).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sophiecarreras
sophiecarreras merged commit 6298cc4 into master Oct 14, 2026
4 of 5 checks passed
@sophiecarreras
sophiecarreras deleted the sdkharness/conformance-slice-1 branch October 14, 2026 17:53
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